diff options
23 files changed, 290 insertions, 26 deletions
diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 627d37bac68..651ab440d8c 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -95,6 +95,7 @@ export default { visibilityLevel: visibilityOptions.PUBLIC, issuesAccessLevel: 20, repositoryAccessLevel: 20, + forkingEnabled: true, mergeRequestsAccessLevel: 20, buildsAccessLevel: 20, wikiAccessLevel: 20, @@ -266,6 +267,16 @@ export default { name="project[project_feature_attributes][merge_requests_access_level]" /> </project-setting-row> + <project-setting-row + label="Forks" + help-text="Allow users to make copies of your repository to a new project" + > + <project-feature-toggle + v-model="forkingEnabled" + :disabled-input="!repositoryEnabled" + name="project[project_setting_attributes][forking_enabled]" + /> + </project-setting-row> <project-setting-row label="Pipelines" help-text="Build, test, and deploy your changes"> <project-feature-setting v-model="buildsAccessLevel" diff --git a/app/assets/javascripts/pages/projects/shared/permissions/mixins/settings_pannel_mixin.js b/app/assets/javascripts/pages/projects/shared/permissions/mixins/settings_pannel_mixin.js index fcbd81416f2..6f522703348 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/mixins/settings_pannel_mixin.js +++ b/app/assets/javascripts/pages/projects/shared/permissions/mixins/settings_pannel_mixin.js @@ -12,11 +12,13 @@ export default { this.buildsAccessLevel = Math.min(this.buildsAccessLevel, value); if (value === 0) { + this.forkingEnabled = false; this.containerRegistryEnabled = false; this.lfsEnabled = false; } } else if (oldValue === 0) { this.mergeRequestsAccessLevel = value; + this.forkingEnabled = true; this.buildsAccessLevel = value; this.containerRegistryEnabled = true; this.lfsEnabled = true; diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb index ac1c4bc7fd3..64e797282a1 100644 --- a/app/controllers/projects/forks_controller.rb +++ b/app/controllers/projects/forks_controller.rb @@ -8,6 +8,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 :check_forking_availability, only: [:new, :create] # rubocop: disable CodeReuse/ActiveRecord def index @@ -58,7 +59,13 @@ class Projects::ForksController < Projects::ApplicationController end # rubocop: enable CodeReuse/ActiveRecord + private + def whitelist_query_limiting Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42335') end + + def check_forking_availability + access_denied!(_('Forking is disabled for this project.')) unless @project.forking_enabled + end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index d4ff72c2314..818a878f5bb 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -389,6 +389,10 @@ class ProjectsController < Projects::ApplicationController snippets_access_level wiki_access_level pages_access_level + ], + + project_setting_attributes: %i[ + forking_enabled ] ] end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 71c9c121e48..0bd9dabce3a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -535,6 +535,7 @@ module ProjectsHelper requestAccessEnabled: !!project.request_access_enabled, issuesAccessLevel: feature.issues_access_level, repositoryAccessLevel: feature.repository_access_level, + forkingEnabled: project.forking_enabled, mergeRequestsAccessLevel: feature.merge_requests_access_level, buildsAccessLevel: feature.builds_access_level, wikiAccessLevel: feature.wiki_access_level, diff --git a/app/models/project.rb b/app/models/project.rb index a6e43efa1f3..c54ce42f6f5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -231,6 +231,7 @@ class Project < ApplicationRecord has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true has_one :project_feature, inverse_of: :project + has_one :project_setting has_one :statistics, class_name: 'ProjectStatistics' has_one :cluster_project, class_name: 'Clusters::Project' @@ -286,6 +287,7 @@ class Project < ApplicationRecord accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature, update_only: true + accepts_nested_attributes_for :project_setting, update_only: true accepts_nested_attributes_for :import_data accepts_nested_attributes_for :auto_devops, update_only: true accepts_nested_attributes_for :ci_cd_settings, update_only: true @@ -305,6 +307,7 @@ class Project < ApplicationRecord delegate :group_runners_enabled, :group_runners_enabled=, :group_runners_enabled?, to: :ci_cd_settings delegate :root_ancestor, to: :namespace, allow_nil: true delegate :last_pipeline, to: :commit, allow_nil: true + delegate :forking_enabled, to: :project_setting, allow_nil: true delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci @@ -606,6 +609,10 @@ class Project < ApplicationRecord super end + def project_setting + super || build_project_setting + end + def all_pipelines if builds_enabled? super diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb new file mode 100644 index 00000000000..a0d7e4c348d --- /dev/null +++ b/app/models/project_setting.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ProjectSetting < ApplicationRecord + belongs_to :project +end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e79bac6bee3..d6a3f440ff3 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -83,6 +83,9 @@ class ProjectPolicy < BasePolicy project.merge_requests_allowing_push_to_user(user).any? end + desc "Project allows forking" + condition(:forking_allowed) { @subject.forking_enabled } + with_scope :global condition(:mirror_available, score: 0) do ::Gitlab::CurrentSettings.current_application_settings.mirror_available @@ -196,7 +199,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 @@ -225,12 +227,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?(:public_user_access) | can?(:reporter_access)) & forking_allowed }.policy do + enable :fork_project + end + rule { owner | admin | guest | group_member }.prevent :request_access rule { ~request_access_enabled }.prevent :request_access diff --git a/changelogs/unreleased/15082-restrict_project_forks.yml b/changelogs/unreleased/15082-restrict_project_forks.yml new file mode 100644 index 00000000000..8b79725c024 --- /dev/null +++ b/changelogs/unreleased/15082-restrict_project_forks.yml @@ -0,0 +1,5 @@ +--- +title: Add an option to configure forking restriction +merge_request: 25437 +author: +type: added diff --git a/db/migrate/20190215151545_create_project_settings.rb b/db/migrate/20190215151545_create_project_settings.rb new file mode 100644 index 00000000000..ba864cbd9d3 --- /dev/null +++ b/db/migrate/20190215151545_create_project_settings.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateProjectSettings < ActiveRecord::Migration[5.0] + DOWNTIME = false + + def change + create_table :project_settings do |t| + t.references :project, + null: false, + index: { unique: true }, + foreign_key: { on_delete: :cascade } + + t.boolean :forking_enabled, + default: true, + null: false + + t.timestamps_with_timezone null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 591758af0e4..19b960d44ac 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2646,6 +2646,14 @@ ActiveRecord::Schema.define(version: 2019_08_06_071559) do t.index ["project_id"], name: "index_project_repository_states_on_project_id", unique: true end + create_table "project_settings", id: :serial, force: :cascade do |t| + t.integer "project_id", null: false + t.boolean "forking_enabled", default: true, null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.index ["project_id"], name: "index_project_settings_on_project_id", unique: true + end + create_table "project_statistics", id: :serial, force: :cascade do |t| t.integer "project_id", null: false t.integer "namespace_id", null: false @@ -3897,6 +3905,7 @@ ActiveRecord::Schema.define(version: 2019_08_06_071559) do add_foreign_key "project_repositories", "projects", on_delete: :cascade add_foreign_key "project_repositories", "shards", on_delete: :restrict add_foreign_key "project_repository_states", "projects", on_delete: :cascade + add_foreign_key "project_settings", "projects", on_delete: :cascade add_foreign_key "project_statistics", "projects", on_delete: :cascade add_foreign_key "project_tracing_settings", "projects", on_delete: :cascade add_foreign_key "projects", "pool_repositories", name: "fk_6e5c14658a", on_delete: :nullify diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index f3888857bb6..5faa2f9a964 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -74,6 +74,7 @@ project_tree: - protected_tags: - :create_access_levels - :project_feature + - :project_setting - :custom_attributes - :prometheus_metrics - :project_badges diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b96ceb970ae..e52becee87e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5032,6 +5032,9 @@ msgstr "" msgid "Forking in progress" msgstr "" +msgid "Forking is disabled for this project." +msgstr "" + msgid "Forking repository" msgstr "" diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index 5ac5279e997..6381faae089 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -12,13 +12,29 @@ describe Projects::ForksController do group.add_owner(user) end + shared_examples 'forking disabled' do + let(:project) { create(:project, :private, :repository) } + + before do + create(:project_setting, { project: project, forking_enabled: false }) + project.add_developer(user) + sign_in(user) + end + + it 'returns with 403' do + subject + + expect(response).to have_gitlab_http_status(403) + end + end + describe 'GET index' do def get_forks get :index, - params: { - namespace_id: project.namespace, - project_id: project - } + params: { + namespace_id: project.namespace, + project_id: project + } end context 'when fork is public' do @@ -85,19 +101,19 @@ describe Projects::ForksController do end describe 'GET new' do - def get_new + subject do get :new, - params: { - namespace_id: project.namespace, - project_id: project - } + params: { + namespace_id: project.namespace, + project_id: project + } end context 'when user is signed in' do it 'responds with status 200' do sign_in(user) - get_new + subject expect(response).to have_gitlab_http_status(200) end @@ -107,21 +123,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 @@ -130,18 +151,34 @@ 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 - it 'passes continue params to the redirect' do - continue_params = { to: '/-/ide/project/path', notice: 'message' } - post_create continue: continue_params + 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 - expect(response).to have_gitlab_http_status(302) - expect(response).to redirect_to(namespace_project_import_path(user.namespace, project, continue: continue_params)) + it 'passes continue params to the redirect' do + 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 @@ -149,10 +186,12 @@ describe Projects::ForksController 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 diff --git a/spec/factories/project_settings.rb b/spec/factories/project_settings.rb new file mode 100644 index 00000000000..c0c5039ab51 --- /dev/null +++ b/spec/factories/project_settings.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :project_setting do + project + end +end diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index ca383da5f5c..f63794a10a9 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -180,7 +180,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 diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb index 2aed402652b..97cc9bcc880 100644 --- a/spec/features/projects/fork_spec.rb +++ b/spec/features/projects/fork_spec.rb @@ -27,6 +27,53 @@ describe 'Project fork' do expect(page).to have_css('a.disabled', text: 'Fork') end + context 'forking enabled / disabled in project settings' do + let(:project) { create(:project, :private, :repository) } + + before do + project.add_developer(user) + + create(:project_setting, + { project: project, + forking_enabled: forking_enabled }) + end + + context 'forking is enabled' do + let(:forking_enabled) { true } + + 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_enabled) { false } + + 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(403) + expect(page).to have_text('Forking is disabled for this project') + end + end + end + it 'forks the project' do visit project_path(project) diff --git a/spec/features/projects/settings/project_settings_spec.rb b/spec/features/projects/settings/project_settings_spec.rb index 7afddc0e712..17197be51da 100644 --- a/spec/features/projects/settings/project_settings_spec.rb +++ b/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_setting_attributes][forking_enabled]"]', visible: :hidden) + forking_enabled_button = find('input[name="project[project_setting_attributes][forking_enabled]"] + button') + + expect(forking_enabled_input.value).to eq('true') + + # 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('false') + end + end + def expect_toggle_state(state) is_collapsed = state == :collapsed diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index fddb5066d6f..6b754b91670 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -323,6 +323,7 @@ project: - environments - deployments - project_feature +- project_setting - auto_devops - pages_domains - authorized_users diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index f0545176a90..fd5fc383a65 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -543,6 +543,12 @@ ProjectFeature: - pages_access_level - created_at - updated_at +ProjectSetting: +- id +- project_id +- forking_enabled +- created_at +- updated_at ProtectedBranch::MergeAccessLevel: - id - protected_branch_id diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 1d7ca85cdd2..9247f508491 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2528,6 +2528,20 @@ describe API::Projects do expect(json_response['message']).to eq('401 Unauthorized') end end + + context 'forking disabled' do + before do + create(:project_setting, + { project: project, forking_enabled: false }) + 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 diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 0c109e26a6a..1c965385f08 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -196,6 +196,19 @@ describe Projects::ForkService do end end end + + context 'when forking is disabled' do + before do + create(:project_setting, + { project: @from_project, forking_enabled: false }) + 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 diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 82010dd283c..75c3a19e134 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -8,8 +8,11 @@ describe Projects::UpdateService do let(:user) { create(:user) } let(:project) do - create(:project, creator: user, namespace: user.namespace) + create(:project, { creator: user, + namespace: user.namespace, + visibility_level: project_visibility_level }) end + let(:project_visibility_level) { Gitlab::VisibilityLevel::PRIVATE } describe '#execute' do let(:gitlab_shell) { Gitlab::Shell.new } @@ -103,6 +106,40 @@ describe Projects::UpdateService do end end + context 'when changing forking enabled' do + subject do + update_project(project, user, + project_setting_attributes: { forking_enabled: forking_enabled } ) + end + + shared_examples 'valid forking_enabled change' do + it 'succeeds' do + expect(subject).to eq({ status: :success }) + end + + it 'updates the project' do + expect { subject }.to change(project, :forking_enabled).to(forking_enabled) + end + end + + context 'enables forking' do + let(:forking_enabled) { true } + + before do + create(:project_setting, + { project: project, forking_enabled: false }) + end + + it_behaves_like 'valid forking_enabled change' + end + + context 'disables forking' do + let(:forking_enabled) { false } + + it_behaves_like 'valid forking_enabled change' + end + end + describe 'when updating project that has forks' do let(:project) { create(:project, :internal) } let(:forked_project) { fork_project(project) } |