From 74c312e973d675fba92d28bfd21d0f7df8b18ae4 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Fri, 14 Oct 2022 08:03:05 +0000 Subject: [PATCH 01/11] Add audit events for fork unlink action --- app/services/projects/unlink_fork_service.rb | 2 ++ .../ee/projects/unlink_fork_service.rb | 25 ++++++++++++++ .../ee/projects/unlink_fork_service_spec.rb | 34 +++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 ee/app/services/ee/projects/unlink_fork_service.rb create mode 100644 ee/spec/services/ee/projects/unlink_fork_service_spec.rb diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index 9eccc16a8b25d9..898421364db3a1 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -60,3 +60,5 @@ def refresh_forks_count(project) end end end + +Projects::UnlinkForkService.prepend_mod_with('Projects::UnlinkForkService') diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb new file mode 100644 index 00000000000000..f44b632193528d --- /dev/null +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module EE + module Projects + module UnlinkForkService + extend ::Gitlab::Utils::Override + + override :execute + + def execute + super + audit_context = { + name: 'remove_fork_relationship', + author: current_user, + scope: project, + target: project, + message: "Project unlinked from #{project.forked_from_project.full_path}", + created_at: DateTime.current + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb new file mode 100644 index 00000000000000..82bc42b3e9f618 --- /dev/null +++ b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::UnlinkForkService, :clean_gitlab_redis_cache do + include ProjectForksHelper + + subject(:unlink_fork) { described_class.new(forked_project, user).execute } + + let_it_be(:project) { create(:project, :public) } + let(:path) { project.full_path } + let!(:forked_project) { fork_project(project, user) } + let(:user) { create(:user) } + + context 'when forked project is unlinked from parent' do + it 'creates an audit event', :aggregate_failures do + expect { unlink_fork }.to change(AuditEvent, :count).by(1) + + expect(AuditEvent.last).to have_attributes( + author: user, + entity_id: user.id, + target_type: project.class.name, + details: { + author_class: user.class.name, + author_name: user.name, + custom_message: "Project unlinked from #{path}", + target_details: forked_project.name, + target_id: forked_project.id, + target_type: forked_project.class.name + } + ) + end + end +end -- GitLab From 7198db68a1f86fec4ef739f4a65b571066f64783 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Fri, 14 Oct 2022 15:55:51 +0000 Subject: [PATCH 02/11] Fix rspec failures --- ee/app/services/ee/projects/unlink_fork_service.rb | 4 +++- ee/spec/services/ee/projects/unlink_fork_service_spec.rb | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb index f44b632193528d..070d1ec03e1ebb 100644 --- a/ee/app/services/ee/projects/unlink_fork_service.rb +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -8,13 +8,15 @@ module UnlinkForkService override :execute def execute + fork_network = @project.fork_network + return unless fork_network super audit_context = { name: 'remove_fork_relationship', author: current_user, scope: project, target: project, - message: "Project unlinked from #{project.forked_from_project.full_path}", + message: "Project unlinked from #{project.forked_from_project&.name}", created_at: DateTime.current } diff --git a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb index 82bc42b3e9f618..8a906b20d388a8 100644 --- a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb +++ b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb @@ -8,7 +8,6 @@ subject(:unlink_fork) { described_class.new(forked_project, user).execute } let_it_be(:project) { create(:project, :public) } - let(:path) { project.full_path } let!(:forked_project) { fork_project(project, user) } let(:user) { create(:user) } @@ -18,12 +17,12 @@ expect(AuditEvent.last).to have_attributes( author: user, - entity_id: user.id, + entity_id: forked_project.id, target_type: project.class.name, details: { author_class: user.class.name, author_name: user.name, - custom_message: "Project unlinked from #{path}", + custom_message: "Project unlinked from #{project&.name}", target_details: forked_project.name, target_id: forked_project.id, target_type: forked_project.class.name -- GitLab From 0ddf59a4596965cd44edbc20c0c025d221aa90ef Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Fri, 14 Oct 2022 16:29:08 +0000 Subject: [PATCH 03/11] Fix robocop failures Add project level audit event for fork relationship removal Changelog: added EE: true --- ee/app/services/ee/projects/unlink_fork_service.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb index 070d1ec03e1ebb..464082a4cdff36 100644 --- a/ee/app/services/ee/projects/unlink_fork_service.rb +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -8,8 +8,10 @@ module UnlinkForkService override :execute def execute - fork_network = @project.fork_network + fork_network = project.fork_network + return unless fork_network + super audit_context = { name: 'remove_fork_relationship', -- GitLab From 78f66cb445c2d3467e4b28a1f5bb6f506156e327 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Thu, 20 Oct 2022 09:53:45 +0000 Subject: [PATCH 04/11] Add event type info --- app/services/projects/unlink_fork_service.rb | 3 +++ ee/app/services/ee/projects/unlink_fork_service.rb | 9 ++------- .../audit_events/types/remove_fork_relationship.yml | 8 ++++++++ ee/spec/services/ee/projects/unlink_fork_service_spec.rb | 4 ++++ 4 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 ee/config/audit_events/types/remove_fork_relationship.yml diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index 898421364db3a1..9d1a34970dafa9 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -51,8 +51,11 @@ def execute [forked_from, @project].compact.each do |project| refresh_forks_count(project) end + log_audit_event end + def log_audit_event; end + private def refresh_forks_count(project) diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb index 464082a4cdff36..5f3df7044a432e 100644 --- a/ee/app/services/ee/projects/unlink_fork_service.rb +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -5,14 +5,9 @@ module Projects module UnlinkForkService extend ::Gitlab::Utils::Override - override :execute + override :log_audit_event - def execute - fork_network = project.fork_network - - return unless fork_network - - super + def log_audit_event audit_context = { name: 'remove_fork_relationship', author: current_user, diff --git a/ee/config/audit_events/types/remove_fork_relationship.yml b/ee/config/audit_events/types/remove_fork_relationship.yml new file mode 100644 index 00000000000000..07a67d9f682018 --- /dev/null +++ b/ee/config/audit_events/types/remove_fork_relationship.yml @@ -0,0 +1,8 @@ +name: remove_fork_relationship +description: Event triggered on successful removal of project's fork relationship +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/272532 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101017 +group: "group::compliance" +milestone: "15.6" +saved_to_database: false +streamed: false diff --git a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb index 8a906b20d388a8..ad7d48c5bd44bf 100644 --- a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb +++ b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb @@ -13,6 +13,10 @@ context 'when forked project is unlinked from parent' do it 'creates an audit event', :aggregate_failures do + expect(::Gitlab::Audit::Auditor) + .to receive(:audit).with( + hash_including({ name: "remove_fork_relationship" })).and_call_original + expect { unlink_fork }.to change(AuditEvent, :count).by(1) expect(AuditEvent.last).to have_attributes( -- GitLab From 90e0ea1eb5af06afa53f9f955362f42a2401d412 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Thu, 20 Oct 2022 17:34:32 +0000 Subject: [PATCH 05/11] Fix failed foss tests --- app/services/projects/unlink_fork_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index 9d1a34970dafa9..c4f75387f26ea1 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -44,6 +44,7 @@ def execute end end end + log_audit_event # rubocop: enable Cop/InBatches # When the project getting out of the network is a node with parent @@ -51,7 +52,6 @@ def execute [forked_from, @project].compact.each do |project| refresh_forks_count(project) end - log_audit_event end def log_audit_event; end -- GitLab From e3a6c6a20e11438ad1e805b2d1dedaeafe5b7f40 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Thu, 27 Oct 2022 04:56:34 +0000 Subject: [PATCH 06/11] Add comment for more context --- app/services/projects/unlink_fork_service.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index c4f75387f26ea1..4f3824c2d70048 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -54,6 +54,7 @@ def execute end end + # Overridden in EE def log_audit_event; end private -- GitLab From b884b3ff5bc585f7630654f590b3a5b6718a501b Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Thu, 27 Oct 2022 09:06:46 +0000 Subject: [PATCH 07/11] Change audit event name format --- ee/app/services/ee/projects/unlink_fork_service.rb | 2 +- ...k_relationship.yml => project_fork_relationship_removed.yml} | 2 +- ee/spec/services/ee/projects/unlink_fork_service_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename ee/config/audit_events/types/{remove_fork_relationship.yml => project_fork_relationship_removed.yml} (88%) diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb index 5f3df7044a432e..caef5795a15429 100644 --- a/ee/app/services/ee/projects/unlink_fork_service.rb +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -9,7 +9,7 @@ module UnlinkForkService def log_audit_event audit_context = { - name: 'remove_fork_relationship', + name: 'project_fork_relationship_removed', author: current_user, scope: project, target: project, diff --git a/ee/config/audit_events/types/remove_fork_relationship.yml b/ee/config/audit_events/types/project_fork_relationship_removed.yml similarity index 88% rename from ee/config/audit_events/types/remove_fork_relationship.yml rename to ee/config/audit_events/types/project_fork_relationship_removed.yml index 07a67d9f682018..2df34690a5801a 100644 --- a/ee/config/audit_events/types/remove_fork_relationship.yml +++ b/ee/config/audit_events/types/project_fork_relationship_removed.yml @@ -1,4 +1,4 @@ -name: remove_fork_relationship +name: project_fork_relationship_removed description: Event triggered on successful removal of project's fork relationship introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/272532 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101017 diff --git a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb index ad7d48c5bd44bf..dbf907755111c2 100644 --- a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb +++ b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb @@ -15,7 +15,7 @@ it 'creates an audit event', :aggregate_failures do expect(::Gitlab::Audit::Auditor) .to receive(:audit).with( - hash_including({ name: "remove_fork_relationship" })).and_call_original + hash_including({ name: "project_fork_relationship_removed" })).and_call_original expect { unlink_fork }.to change(AuditEvent, :count).by(1) -- GitLab From 96375314104f3481c559f3fb73f8a5e5a666c727 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Fri, 28 Oct 2022 10:12:18 +0000 Subject: [PATCH 08/11] Adjust event type flags --- .../audit_events/types/project_fork_relationship_removed.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/config/audit_events/types/project_fork_relationship_removed.yml b/ee/config/audit_events/types/project_fork_relationship_removed.yml index 2df34690a5801a..9a81ff95c96209 100644 --- a/ee/config/audit_events/types/project_fork_relationship_removed.yml +++ b/ee/config/audit_events/types/project_fork_relationship_removed.yml @@ -4,5 +4,5 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/272532 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101017 group: "group::compliance" milestone: "15.6" -saved_to_database: false -streamed: false +saved_to_database: true +streamed: true -- GitLab From cc812bb7ee76bd40feeeafe2b142f5259ca16121 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Fri, 28 Oct 2022 16:29:16 +0000 Subject: [PATCH 09/11] Modify log_audit_event entry point --- app/services/projects/unlink_fork_service.rb | 4 ---- ee/app/services/ee/projects/unlink_fork_service.rb | 7 ++++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index 4f3824c2d70048..898421364db3a1 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -44,7 +44,6 @@ def execute end end end - log_audit_event # rubocop: enable Cop/InBatches # When the project getting out of the network is a node with parent @@ -54,9 +53,6 @@ def execute end end - # Overridden in EE - def log_audit_event; end - private def refresh_forks_count(project) diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb index caef5795a15429..a10351c6e9650f 100644 --- a/ee/app/services/ee/projects/unlink_fork_service.rb +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -5,7 +5,12 @@ module Projects module UnlinkForkService extend ::Gitlab::Utils::Override - override :log_audit_event + override :execute + def execute + super.tap do |result| + log_audit_event if result + end + end def log_audit_event audit_context = { -- GitLab From b51e1e9cce43517ab2f62ca94f8f835fa3928750 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Sat, 29 Oct 2022 03:54:22 +0000 Subject: [PATCH 10/11] Add unlink service failure test case --- .../ee/projects/unlink_fork_service_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb index dbf907755111c2..b11b7a49cf43aa 100644 --- a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb +++ b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb @@ -34,4 +34,17 @@ ) end end + + context 'when unlink fork service fails' do + before do + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:execute) + .and_return({ status: :error }) + end + end + + it 'does not create an audit event' do + expect { subject }.not_to change(AuditEvent, :count) + end + end end -- GitLab From c21d70910f1c1b4e7c145d09ee5dd5908b0920ef Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Tue, 1 Nov 2022 04:06:09 +0000 Subject: [PATCH 11/11] Add source failure test case and refactor result --- .../ee/projects/unlink_fork_service.rb | 2 +- .../ee/projects/unlink_fork_service_spec.rb | 68 +++++++++++++------ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/ee/app/services/ee/projects/unlink_fork_service.rb b/ee/app/services/ee/projects/unlink_fork_service.rb index a10351c6e9650f..0aedf1e8b72b50 100644 --- a/ee/app/services/ee/projects/unlink_fork_service.rb +++ b/ee/app/services/ee/projects/unlink_fork_service.rb @@ -8,7 +8,7 @@ module UnlinkForkService override :execute def execute super.tap do |result| - log_audit_event if result + log_audit_event if result.present? end end diff --git a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb index b11b7a49cf43aa..7509c530624161 100644 --- a/ee/spec/services/ee/projects/unlink_fork_service_spec.rb +++ b/ee/spec/services/ee/projects/unlink_fork_service_spec.rb @@ -2,16 +2,17 @@ require 'spec_helper' -RSpec.describe Projects::UnlinkForkService, :clean_gitlab_redis_cache do +RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_caching do include ProjectForksHelper subject(:unlink_fork) { described_class.new(forked_project, user).execute } let_it_be(:project) { create(:project, :public) } - let!(:forked_project) { fork_project(project, user) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } context 'when forked project is unlinked from parent' do + let!(:forked_project) { fork_project(project, user) } + it 'creates an audit event', :aggregate_failures do expect(::Gitlab::Audit::Auditor) .to receive(:audit).with( @@ -19,29 +20,52 @@ expect { unlink_fork }.to change(AuditEvent, :count).by(1) - expect(AuditEvent.last).to have_attributes( - author: user, - entity_id: forked_project.id, - target_type: project.class.name, - details: { - author_class: user.class.name, - author_name: user.name, - custom_message: "Project unlinked from #{project&.name}", - target_details: forked_project.name, - target_id: forked_project.id, - target_type: forked_project.class.name - } - ) + expect(AuditEvent.last).to have_attributes({ + author: user, + entity_id: forked_project.id, + target_type: project.class.name, + details: { + author_class: user.class.name, + author_name: user.name, + custom_message: "Project unlinked from #{project.name}", + target_details: forked_project.name, + target_id: forked_project.id, + target_type: forked_project.class.name + } + }) end - end - context 'when unlink fork service fails' do - before do - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:execute) - .and_return({ status: :error }) + context 'when forked project does not exist' do + before do + project.destroy! + end + + it 'creates an audit event', :aggregate_failures do + expect(::Gitlab::Audit::Auditor) + .to receive(:audit).with( + hash_including({ name: "project_fork_relationship_removed" })).and_call_original + + expect { unlink_fork }.to change(AuditEvent, :count).by(1) + + expect(AuditEvent.last).to have_attributes({ + author: user, + entity_id: forked_project.id, + target_type: project.class.name, + details: { + author_class: user.class.name, + author_name: user.name, + custom_message: "Project unlinked from ", + target_details: forked_project.name, + target_id: forked_project.id, + target_type: forked_project.class.name + } + }) end end + end + + context 'when no unlinking is performed' do + let(:forked_project) { project } it 'does not create an audit event' do expect { subject }.not_to change(AuditEvent, :count) -- GitLab