Merge branch 'if-restrict_project_forks' into 'master'

Restrict project forking

See merge request gitlab-org/gitlab!17988

app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue

@@ -104,6 +104,7 @@ export default {
       visibilityLevel: visibilityOptions.PUBLIC,
       issuesAccessLevel: 20,
       repositoryAccessLevel: 20,
+      forkingAccessLevel: 20,
       mergeRequestsAccessLevel: 20,
       buildsAccessLevel: 20,
       wikiAccessLevel: 20,
@@ -300,6 +301,19 @@ export default {
             name="project[project_feature_attributes][merge_requests_access_level]"
           />
         </project-setting-row>
+        <project-setting-row
+          :label="s__('ProjectSettings|Forks')"
+          :help-text="
+            s__('ProjectSettings|Allow users to make copies of your repository to a new project')
+          "
+        >
+          <project-feature-setting
+            v-model="forkingAccessLevel"
+            :options="featureAccessLevelOptions"
+            :disabled-input="!repositoryEnabled"
+            name="project[project_feature_attributes][forking_access_level]"
+          />
+        </project-setting-row>
         <project-setting-row
           :label="s__('ProjectSettings|Pipelines')"
           :help-text="s__('ProjectSettings|Build, test, and deploy your changes')"

app/controllers/projects/forks_controller.rb

@@ -9,6 +9,7 @@ class Projects::ForksController < Projects::ApplicationController
   before_action :require_non_empty_project
   before_action :authorize_download_code!
   before_action :authenticate_user!, only: [:new, :create]
+  before_action :authorize_fork_project!, only: [:new, :create]
 
   # rubocop: disable CodeReuse/ActiveRecord
   def index
@@ -61,6 +62,8 @@ class Projects::ForksController < Projects::ApplicationController
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  private
+
   def whitelist_query_limiting
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42335')
   end

app/controllers/projects_controller.rb

@@ -391,6 +391,7 @@ class ProjectsController < Projects::ApplicationController
       project_feature_attributes: %i[
         builds_access_level
         issues_access_level
+        forking_access_level
         merge_requests_access_level
         repository_access_level
         snippets_access_level

app/helpers/projects_helper.rb

@@ -563,6 +563,7 @@ module ProjectsHelper
       requestAccessEnabled: !!project.request_access_enabled,
       issuesAccessLevel: feature.issues_access_level,
       repositoryAccessLevel: feature.repository_access_level,
+      forkingAccessLevel: feature.forking_access_level,
       mergeRequestsAccessLevel: feature.merge_requests_access_level,
       buildsAccessLevel: feature.builds_access_level,
       wikiAccessLevel: feature.wiki_access_level,

app/models/concerns/project_features_compatibility.rb

@@ -50,6 +50,10 @@ module ProjectFeaturesCompatibility
     write_feature_attribute_string(:merge_requests_access_level, value)
   end
 
+  def forking_access_level=(value)
+    write_feature_attribute_string(:forking_access_level, value)
+  end
+
   def issues_access_level=(value)
     write_feature_attribute_string(:issues_access_level, value)
   end

app/models/project.rb

@@ -317,10 +317,12 @@ class Project < ApplicationRecord
   accepts_nested_attributes_for :metrics_setting, update_only: true, allow_destroy: true
   accepts_nested_attributes_for :grafana_integration, update_only: true, allow_destroy: true
 
-  delegate :feature_available?, :builds_enabled?, :wiki_enabled?, :merge_requests_enabled?,
-    :issues_enabled?, :pages_enabled?, :public_pages?, :private_pages?,
-    :merge_requests_access_level, :issues_access_level, :wiki_access_level,
-    :snippets_access_level, :builds_access_level, :repository_access_level,
+  delegate :feature_available?, :builds_enabled?, :wiki_enabled?,
+    :merge_requests_enabled?, :forking_enabled?, :issues_enabled?,
+    :pages_enabled?, :public_pages?, :private_pages?,
+    :merge_requests_access_level, :forking_access_level, :issues_access_level,
+    :wiki_access_level, :snippets_access_level, :builds_access_level,
+    :repository_access_level,
     to: :project_feature, allow_nil: true
   delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?,
     prefix: :import, to: :import_state, allow_nil: true

app/models/project_feature.rb

@@ -22,7 +22,7 @@ class ProjectFeature < ApplicationRecord
   ENABLED  = 20
   PUBLIC   = 30
 
-  FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze
+  FEATURES = %i(issues forking merge_requests wiki snippets builds repository pages).freeze
   PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze
   PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
   STRING_OPTIONS = HashWithIndifferentAccess.new({
@@ -92,6 +92,7 @@ class ProjectFeature < ApplicationRecord
 
   default_value_for :builds_access_level,         value: ENABLED, allows_nil: false
   default_value_for :issues_access_level,         value: ENABLED, allows_nil: false
+  default_value_for :forking_access_level,        value: ENABLED, allows_nil: false
   default_value_for :merge_requests_access_level, value: ENABLED, allows_nil: false
   default_value_for :snippets_access_level,       value: ENABLED, allows_nil: false
   default_value_for :wiki_access_level,           value: ENABLED, allows_nil: false
@@ -132,6 +133,10 @@ class ProjectFeature < ApplicationRecord
     merge_requests_access_level > DISABLED
   end
 
+  def forking_enabled?
+    forking_access_level > DISABLED
+  end
+
   def issues_enabled?
     issues_access_level > DISABLED
   end

app/policies/project_policy.rb

@@ -83,6 +83,11 @@ class ProjectPolicy < BasePolicy
     project.merge_requests_allowing_push_to_user(user).any?
   end
 
+  with_scope :subject
+  condition(:forking_allowed) do
+    @subject.feature_available?(:forking, @user)
+  end
+
   with_scope :global
   condition(:mirror_available, score: 0) do
     ::Gitlab::CurrentSettings.current_application_settings.mirror_available
@@ -203,7 +208,6 @@ class ProjectPolicy < BasePolicy
     enable :download_code
     enable :read_statistics
     enable :download_wiki_code
-    enable :fork_project
     enable :create_project_snippet
     enable :update_issue
     enable :reopen_issue
@@ -232,12 +236,15 @@ class ProjectPolicy < BasePolicy
     enable :public_access
     enable :guest_access
 
-    enable :fork_project
     enable :build_download_code
     enable :build_read_container_image
     enable :request_access
   end
 
+  rule { can?(:download_code) & forking_allowed }.policy do
+    enable :fork_project
+  end
+
   rule { owner | admin | guest | group_member }.prevent :request_access
   rule { ~request_access_enabled }.prevent :request_access
 

changelogs/unreleased/15082-restrict_project_forks.yml

+---
+title: Add an option to configure forking restriction
+merge_request: 17988
+author:
+type: added

db/migrate/20200104113850_add_forking_access_level_to_project_feature.rb

+# frozen_string_literal: true
+
+class AddForkingAccessLevelToProjectFeature < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :project_features, :forking_access_level, :integer
+  end
+end

db/schema.rb

@@ -3167,6 +3167,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_204949) do
     t.datetime "updated_at"
     t.integer "repository_access_level", default: 20, null: false
     t.integer "pages_access_level", null: false
+    t.integer "forking_access_level"
     t.index ["project_id"], name: "index_project_features_on_project_id", unique: true
   end
 

doc/user/project/settings/index.md

@@ -43,6 +43,7 @@ Use the switches to enable or disable the following features:
 | **Issues**                        | ✓                         | Activates the GitLab issues tracker                                                                                                                                                            |
 | **Repository**                    | ✓                         | Enables [repository](../repository/) functionality                                                                                                                                             |
 | **Merge Requests**                | ✓                         | Enables [merge request](../merge_requests/) functionality; also see [Merge request settings](#merge-request-settings)                                                                          |
+| **Forks**                         | ✓                         | Enables [forking](../index.md#fork-a-project) functionality                                                                                                                                    |
 | **Pipelines**                     | ✓                         | Enables [CI/CD](../../../ci/README.md) functionality                                                                                                                                           |
 | **Container Registry**            |                           | Activates a [registry](../../packages/container_registry/) for your docker images                                                                                                              |
 | **Git Large File Storage**        |                           | Enables the use of [large files](../../../administration/lfs/manage_large_binaries_with_git_lfs.md#git-lfs)                                                                                    |

ee/lib/ee/audit/project_feature_changes_auditor.rb

@@ -6,6 +6,7 @@ module EE
       attr_accessor :project
 
       COLUMNS = [:merge_requests_access_level,
+                 :forking_access_level,
                  :issues_access_level,
                  :wiki_access_level,
                  :snippets_access_level,

locale/gitlab.pot

@@ -14360,6 +14360,9 @@ msgstr ""
 msgid "ProjectSettings|All discussions must be resolved"
 msgstr ""
 
+msgid "ProjectSettings|Allow users to make copies of your repository to a new project"
+msgstr ""
+
 msgid "ProjectSettings|Allow users to request access"
 msgstr ""
 
@@ -14420,6 +14423,9 @@ msgstr ""
 msgid "ProjectSettings|Fast-forward merges only"
 msgstr ""
 
+msgid "ProjectSettings|Forks"
+msgstr ""
+
 msgid "ProjectSettings|Git Large File Storage"
 msgstr ""
 

spec/controllers/projects/forks_controller_spec.rb

@@ -12,6 +12,21 @@ describe Projects::ForksController do
     group.add_owner(user)
   end
 
+  shared_examples 'forking disabled' do
+    let(:project) { create(:project, :private, :repository, :forking_disabled) }
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    it 'returns with 404' do
+      subject
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+
   describe 'GET index' do
     def get_forks(search: nil)
       get :index,
@@ -138,7 +153,7 @@ describe Projects::ForksController do
   end
 
   describe 'GET new' do
-    def get_new
+    subject do
       get :new,
           params: {
             namespace_id: project.namespace,
@@ -150,7 +165,7 @@ describe Projects::ForksController do
       it 'responds with status 200' do
         sign_in(user)
 
-        get_new
+        subject
 
         expect(response).to have_gitlab_http_status(200)
       end
@@ -160,21 +175,26 @@ describe Projects::ForksController do
       it 'redirects to the sign-in page' do
         sign_out(user)
 
-        get_new
+        subject
 
         expect(response).to redirect_to(new_user_session_path)
       end
     end
+
+    it_behaves_like 'forking disabled'
   end
 
   describe 'POST create' do
-    def post_create(params = {})
-      post :create,
-        params: {
+    let(:params) do
+      {
         namespace_id: project.namespace,
         project_id: project,
         namespace_key: user.namespace.id
-        }.merge(params)
+      }
+    end
+
+    subject do
+      post :create, params: params
     end
 
     context 'when user is signed in' do
@@ -183,29 +203,47 @@ describe Projects::ForksController do
       end
 
       it 'responds with status 302' do
-        post_create
+        subject
 
         expect(response).to have_gitlab_http_status(302)
         expect(response).to redirect_to(namespace_project_import_path(user.namespace, project))
       end
 
+      context 'continue params' do
+        let(:params) do
+          {
+            namespace_id: project.namespace,
+            project_id: project,
+            namespace_key: user.namespace.id,
+            continue: continue_params
+          }
+        end
+        let(:continue_params) do
+          {
+            to: '/-/ide/project/path',
+            notice: 'message'
+          }
+        end
+
         it 'passes continue params to the redirect' do
-        continue_params = { to: '/-/ide/project/path', notice: 'message' }
-        post_create continue: continue_params
+          subject
 
           expect(response).to have_gitlab_http_status(302)
           expect(response).to redirect_to(namespace_project_import_path(user.namespace, project, continue: continue_params))
         end
       end
+    end
 
     context 'when user is not signed in' do
       it 'redirects to the sign-in page' do
         sign_out(user)
 
-        post_create
+        subject
 
         expect(response).to redirect_to(new_user_session_path)
       end
     end
+
+    it_behaves_like 'forking disabled'
   end
 end

spec/factories/projects.rb

@@ -25,6 +25,7 @@ FactoryBot.define do
       builds_access_level { ProjectFeature::ENABLED }
       snippets_access_level { ProjectFeature::ENABLED }
       issues_access_level { ProjectFeature::ENABLED }
+      forking_access_level { ProjectFeature::ENABLED }
       merge_requests_access_level { ProjectFeature::ENABLED }
       repository_access_level { ProjectFeature::ENABLED }
       pages_access_level do
@@ -48,6 +49,7 @@ FactoryBot.define do
         builds_access_level: builds_access_level,
         snippets_access_level: evaluator.snippets_access_level,
         issues_access_level: evaluator.issues_access_level,
+        forking_access_level: evaluator.forking_access_level,
         merge_requests_access_level: merge_requests_access_level,
         repository_access_level: evaluator.repository_access_level
       }
@@ -264,6 +266,9 @@ FactoryBot.define do
     trait(:issues_disabled)         { issues_access_level { ProjectFeature::DISABLED } }
     trait(:issues_enabled)          { issues_access_level { ProjectFeature::ENABLED } }
     trait(:issues_private)          { issues_access_level { ProjectFeature::PRIVATE } }
+    trait(:forking_disabled)         { forking_access_level { ProjectFeature::DISABLED } }
+    trait(:forking_enabled)          { forking_access_level { ProjectFeature::ENABLED } }
+    trait(:forking_private)          { forking_access_level { ProjectFeature::PRIVATE } }
     trait(:merge_requests_enabled)  { merge_requests_access_level { ProjectFeature::ENABLED } }
     trait(:merge_requests_disabled) { merge_requests_access_level { ProjectFeature::DISABLED } }
     trait(:merge_requests_private)  { merge_requests_access_level { ProjectFeature::PRIVATE } }

spec/features/projects/features_visibility_spec.rb

@@ -186,7 +186,7 @@ describe 'Edit Project Settings' do
         click_button "Save changes"
       end
 
-      expect(find(".sharing-permissions")).to have_selector(".project-feature-toggle.is-disabled", count: 2)
+      expect(find(".sharing-permissions")).to have_selector(".project-feature-toggle.is-disabled", count: 3)
     end
 
     it "shows empty features project homepage" do

spec/features/projects/fork_spec.rb

@@ -27,6 +27,89 @@ describe 'Project fork' do
     expect(page).to have_css('a.disabled', text: 'Fork')
   end
 
+  context 'forking enabled / disabled in project settings' do
+    before do
+      project.project_feature.update_attribute(
+        :forking_access_level, forking_access_level)
+    end
+
+    context 'forking is enabled' do
+      let(:forking_access_level) { ProjectFeature::ENABLED }
+
+      it 'enables fork button' do
+        visit project_path(project)
+
+        expect(page).to have_css('a', text: 'Fork')
+        expect(page).not_to have_css('a.disabled', text: 'Fork')
+      end
+
+      it 'renders new project fork page' do
+        visit new_project_fork_path(project)
+
+        expect(page.status_code).to eq(200)
+        expect(page).to have_text(' Select a namespace to fork the project ')
+      end
+    end
+
+    context 'forking is disabled' do
+      let(:forking_access_level) { ProjectFeature::DISABLED }
+
+      it 'does not render fork button' do
+        visit project_path(project)
+
+        expect(page).not_to have_css('a', text: 'Fork')
+      end
+
+      it 'does not render new project fork page' do
+        visit new_project_fork_path(project)
+
+        expect(page.status_code).to eq(404)
+      end
+    end
+
+    context 'forking is private' do
+      let(:forking_access_level) { ProjectFeature::PRIVATE }
+
+      before do
+        project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+      end
+
+      context 'user is not a team member' do
+        it 'does not render fork button' do
+          visit project_path(project)
+
+          expect(page).not_to have_css('a', text: 'Fork')
+        end
+
+        it 'does not render new project fork page' do
+          visit new_project_fork_path(project)
+
+          expect(page.status_code).to eq(404)
+        end
+      end
+
+      context 'user is a team member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'enables fork button' do
+          visit project_path(project)
+
+          expect(page).to have_css('a', text: 'Fork')
+          expect(page).not_to have_css('a.disabled', text: 'Fork')
+        end
+
+        it 'renders new project fork page' do
+          visit new_project_fork_path(project)
+
+          expect(page.status_code).to eq(200)
+          expect(page).to have_text(' Select a namespace to fork the project ')
+        end
+      end
+    end
+  end
+
   it 'forks the project', :sidekiq_might_not_need_inline do
     visit project_path(project)
 

spec/features/projects/settings/project_settings_spec.rb

@@ -34,6 +34,26 @@ describe 'Projects settings' do
     expect_toggle_state(:expanded)
   end
 
+  context 'forking enabled', :js do
+    it 'toggles forking enabled / disabled' do
+      visit edit_project_path(project)
+
+      forking_enabled_input = find('input[name="project[project_feature_attributes][forking_access_level]"]', visible: :hidden)
+      forking_enabled_button = find('input[name="project[project_feature_attributes][forking_access_level]"] + label > button')
+
+      expect(forking_enabled_input.value).to eq('20')
+
+      # disable by clicking toggle
+      forking_enabled_button.click
+      page.within('.sharing-permissions') do
+        find('input[value="Save changes"]').click
+      end
+      wait_for_requests
+
+      expect(forking_enabled_input.value).to eq('0')
+    end
+  end
+
   def expect_toggle_state(state)
     is_collapsed = state == :collapsed
 

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -545,6 +545,7 @@ ProjectFeature:
 - id
 - project_id
 - merge_requests_access_level
+- forking_access_level
 - issues_access_level
 - wiki_access_level
 - snippets_access_level

spec/requests/api/projects_spec.rb

@@ -2858,6 +2858,20 @@ describe API::Projects do
         expect(json_response['message']).to eq('401 Unauthorized')
       end
     end
+
+    context 'forking disabled' do
+      before do
+        project.project_feature.update_attribute(
+          :forking_access_level, ProjectFeature::DISABLED)
+      end
+
+      it 'denies project to be forked' do
+        post api("/projects/#{project.id}/fork", admin)
+
+        expect(response).to have_gitlab_http_status(409)
+        expect(json_response['message']['forked_from_project_id']).to eq(['is forbidden'])
+      end
+    end
   end
 
   describe 'POST /projects/:id/housekeeping' do

spec/services/projects/fork_service_spec.rb

@@ -224,6 +224,19 @@ describe Projects::ForkService do
           end
         end
       end
+
+      context 'when forking is disabled' do
+        before do
+          @from_project.project_feature.update_attribute(
+            :forking_access_level, ProjectFeature::DISABLED)
+        end
+
+        it 'fails' do
+          to_project = fork_project(@from_project, @to_user, namespace: @to_user.namespace)
+
+          expect(to_project.errors[:forked_from_project_id]).to eq(['is forbidden'])
+        end
+      end
     end
 
     describe 'fork to namespace' do