From 4a4a17bc4753a55bae537e712b9c151bf7944e36 Mon Sep 17 00:00:00 2001 From: Olaoluwa Oluro Date: Mon, 10 Mar 2025 14:21:38 +0000 Subject: [PATCH] Add `organization_id` validations to fork_networks This adds code validations to support the introduction of `organization_id` in `fork_networks` Changelog: added --- app/models/fork_network.rb | 11 +++++ app/services/projects/fork_service.rb | 4 ++ .../projects/overwrite_project_service.rb | 1 + locale/gitlab.pot | 6 +++ spec/models/fork_network_spec.rb | 32 ++++++++++++++ spec/requests/api/projects_spec.rb | 12 +++++ .../overwrite_project_service_spec.rb | 44 +++++++++++++++++++ 7 files changed, 110 insertions(+) diff --git a/app/models/fork_network.rb b/app/models/fork_network.rb index 70d5bb25774cab..8fcf2756bf7322 100644 --- a/app/models/fork_network.rb +++ b/app/models/fork_network.rb @@ -7,6 +7,8 @@ class ForkNetwork < ApplicationRecord has_many :fork_network_members has_many :projects, through: :fork_network_members + validate :organization_match + after_create :add_root_as_member, if: :root_project def add_root_as_member @@ -20,4 +22,13 @@ def find_forks_in(other_projects) def merge_requests MergeRequest.where(target_project: projects) end + + private + + def organization_match + return unless root_project + return if root_project.organization_id == organization_id + + errors.add(:organization_id, _("must match the root project organization's ID")) + end end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 476d1630efae5e..add572255b1f1f 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -42,6 +42,10 @@ def link_existing_project(fork_to_project) return ServiceResponse.error(message: _('Target project cannot be equal to source project'), reason: :self_fork) end + if fork_to_project.organization_id != fork_network.organization_id + return ServiceResponse.error(message: _('Target project must belong to source project organization'), reason: :fork_organization_mismatch) + end + build_fork_network_member(fork_to_project) if link_fork_network(fork_to_project) diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index 3a48e9d18f36bc..31f8c5b07eb1be 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -78,6 +78,7 @@ def rename_project(target_project, name, path) def add_source_project_to_fork_network(source_project) return if source_project == @project return unless fork_network + return if fork_network.organization_id != source_project.organization_id # Because they have moved all references in the fork network from the source_project # we won't be able to query the database (only through its cached data), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2d822b6ee1a0cb..8ec01b5bbb1658 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -59952,6 +59952,9 @@ msgstr "" msgid "Target project cannot be equal to source project" msgstr "" +msgid "Target project must belong to source project organization" +msgstr "" + msgid "Target roles" msgstr "" @@ -72736,6 +72739,9 @@ msgstr "" msgid "must match the parent organization's ID" msgstr "" +msgid "must match the root project organization's ID" +msgstr "" + msgid "must not be a placeholder email" msgstr "" diff --git a/spec/models/fork_network_spec.rb b/spec/models/fork_network_spec.rb index 6b7484262cb43e..dcd9920f13fd84 100644 --- a/spec/models/fork_network_spec.rb +++ b/spec/models/fork_network_spec.rb @@ -7,6 +7,38 @@ describe "validations" do it { is_expected.to belong_to(:organization) } + + describe "#organization_match" do + let_it_be(:organization) { create(:organization) } + let_it_be(:project) { create(:project, organization: organization) } + + context "when organization_id matches root_project's organization_id" do + let(:fork_network) { build(:fork_network, root_project: project, organization: organization) } + + it "is valid" do + expect(fork_network).to be_valid + end + end + + context "when organization_id does not match root_project's organization_id" do + let_it_be(:different_organization) { create(:organization) } + + let(:fork_network) { build(:fork_network, root_project: project, organization: different_organization) } + + it "is not valid" do + expect(fork_network).not_to be_valid + expect(fork_network.errors[:organization_id]).to include("must match the root project organization's ID") + end + end + + context "when root_project is nil" do + let(:fork_network) { build(:fork_network, root_project: nil, organization: organization) } + + it "is valid" do + expect(fork_network).to be_valid + end + end + end end describe '#add_root_as_member' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 7f7843a29627d5..29afbc765252b7 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3635,6 +3635,18 @@ def failure_message(diff) expect(json_response['message']).to eq 'Target project cannot be equal to source project' end end + + context 'when fork target and source project organization are not the same' do + let_it_be(:organization) { create(:organization) } + let_it_be(:project_fork_target_different_organization) { create(:project, organization: organization) } + + it 'returns an error' do + post api("/projects/#{project_fork_target_different_organization.id}/fork/#{project_fork_source.id}", admin, admin_mode: true) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq 'Target project must belong to source project organization' + end + end end end diff --git a/spec/services/projects/overwrite_project_service_spec.rb b/spec/services/projects/overwrite_project_service_spec.rb index 99be630d6f6ef6..3e3de6f4062a7b 100644 --- a/spec/services/projects/overwrite_project_service_spec.rb +++ b/spec/services/projects/overwrite_project_service_spec.rb @@ -245,5 +245,49 @@ expect(project_from.fork_network_member).to be_nil end end + + context 'fork network membership behavior' do + shared_examples 'does not modify ForkNetworkMember' do + it 'does not add the source project to the fork network' do + expect { subject.execute(project_from) }.not_to change { + ForkNetworkMember.count + } + end + end + + context 'when fork network conditions are met' do + it 'adds the source project to the fork network' do + expect { subject.execute(project_from) }.to change { + ForkNetworkMember.count + }.by(1) + end + end + + context 'when source project is the same as target project' do + it_behaves_like 'does not modify ForkNetworkMember' do + let(:project_from) { project_to } + end + end + + context 'when fork_network does not exist' do + it_behaves_like 'does not modify ForkNetworkMember' do + before do + allow(subject).to receive(:fork_network).and_return(nil) + end + end + end + + context 'when organizations do not match' do + it_behaves_like 'does not modify ForkNetworkMember' do + before do + other_organization = create(:organization) + allow(project_from).to receive(:organization_id).and_return(other_organization.id) + + # Stub rename_project to not interrupt test flow + allow(subject).to receive(:rename_project).and_return({ status: :success }) + end + end + end + end end end -- GitLab