From c1ed6d2b35153c613a11ea0cd00b63958db2b79e Mon Sep 17 00:00:00 2001
From: Sashi Kumar Kumaresan <skumar@gitlab.com>
Date: Tue, 3 Jan 2023 18:49:50 +0000
Subject: [PATCH] Delete project specific licenses when license policy is
deleted
Merge branch 'security-shared-project-license-policy-15-6' into 'master'
See merge request gitlab-org/security/gitlab!2894
Changelog: security
---
ee/app/models/software_license_policy.rb | 1 +
.../delete_service.rb | 18 +++++++
ee/lib/api/managed_licenses.rb | 4 +-
.../models/software_license_policy_spec.rb | 8 +++
ee/spec/requests/api/managed_licenses_spec.rb | 1 +
.../delete_service_spec.rb | 53 +++++++++++++++++++
6 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644 ee/app/services/software_license_policies/delete_service.rb
create mode 100644 ee/spec/services/software_license_policies/delete_service_spec.rb
diff --git a/ee/app/models/software_license_policy.rb b/ee/app/models/software_license_policy.rb
index 66393086cc43d..9552102ca0010 100644
--- a/ee/app/models/software_license_policy.rb
+++ b/ee/app/models/software_license_policy.rb
@@ -33,6 +33,7 @@ class SoftwareLicensePolicy < ApplicationRecord
scope :with_license, -> { joins(:software_license) }
scope :including_license, -> { includes(:software_license) }
scope :unreachable_limit, -> { limit(1_000) }
+ scope :count_for_software_license, ->(software_license_id) { where(software_license_id: software_license_id).count }
scope :with_license_by_name, -> (license_name) do
with_license.where(SoftwareLicense.arel_table[:name].lower.in(Array(license_name).map(&:downcase)))
diff --git a/ee/app/services/software_license_policies/delete_service.rb b/ee/app/services/software_license_policies/delete_service.rb
new file mode 100644
index 0000000000000..d1a8e91e89caa
--- /dev/null
+++ b/ee/app/services/software_license_policies/delete_service.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module SoftwareLicensePolicies
+ class DeleteService < ::BaseService
+ def execute(software_license_policy)
+ SoftwareLicensePolicy.transaction do
+ software_license = SoftwareLicense.find(software_license_policy.software_license_id)
+
+ software_license_policy.destroy!
+
+ if software_license.spdx_identifier.nil? &&
+ SoftwareLicensePolicy.count_for_software_license(software_license.id) == 0
+ software_license.destroy!
+ end
+ end
+ end
+ end
+end
diff --git a/ee/lib/api/managed_licenses.rb b/ee/lib/api/managed_licenses.rb
index df3afdbc62e67..b4ad074cc8cfd 100644
--- a/ee/lib/api/managed_licenses.rb
+++ b/ee/lib/api/managed_licenses.rb
@@ -146,7 +146,9 @@ def authorize_can_admin!
authorize_can_admin!
not_found!('SoftwareLicensePolicy') unless software_license_policy
- software_license_policy.destroy!
+ SoftwareLicensePolicies::DeleteService
+ .new(user_project, current_user)
+ .execute(software_license_policy)
no_content!
end
diff --git a/ee/spec/models/software_license_policy_spec.rb b/ee/spec/models/software_license_policy_spec.rb
index 616c0d7624c96..eeafe3969606a 100644
--- a/ee/spec/models/software_license_policy_spec.rb
+++ b/ee/spec/models/software_license_policy_spec.rb
@@ -45,6 +45,14 @@
it { expect(described_class.by_spdx(SecureRandom.uuid)).to be_empty }
end
+ describe '.count_for_software_license' do
+ let!(:mit) { create(:software_license, :mit) }
+ let!(:mit_policy1) { create(:software_license_policy, software_license: mit) }
+ let!(:mit_policy2) { create(:software_license_policy, software_license: mit) }
+
+ it { expect(described_class.count_for_software_license(mit.id)).to eq(2) }
+ end
+
describe "#name" do
specify { expect(subject.name).to eql(subject.software_license.name) }
end
diff --git a/ee/spec/requests/api/managed_licenses_spec.rb b/ee/spec/requests/api/managed_licenses_spec.rb
index 60961eeb438c8..d3e99ef852669 100644
--- a/ee/spec/requests/api/managed_licenses_spec.rb
+++ b/ee/spec/requests/api/managed_licenses_spec.rb
@@ -271,6 +271,7 @@
expect(response).to have_gitlab_http_status(:no_content)
end.to change { project.software_license_policies.count }.by(-1)
+ .and change { SoftwareLicense.count }.by(-1)
end
it 'responds with 404 Not Found if requesting non-existing managed license' do
diff --git a/ee/spec/services/software_license_policies/delete_service_spec.rb b/ee/spec/services/software_license_policies/delete_service_spec.rb
new file mode 100644
index 0000000000000..e855b98accd77
--- /dev/null
+++ b/ee/spec/services/software_license_policies/delete_service_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SoftwareLicensePolicies::DeleteService, feature_category: :security_policy_management do
+ subject(:service) { described_class.new(project, user) }
+
+ let_it_be(:project) { create(:project) }
+
+ let(:user) do
+ create(:user).tap do |u|
+ project.add_maintainer(u)
+ end
+ end
+
+ let(:software_license) { create(:software_license) }
+ let(:software_license_policy) { create(:software_license_policy, :denied, software_license: software_license) }
+
+ describe '#execute' do
+ context 'when software_license has one software_license_policy' do
+ it 'deletes software_license_policy and software_license' do
+ service.execute(software_license_policy)
+
+ expect { software_license_policy.reload }.to raise_error(ActiveRecord::RecordNotFound)
+ expect { software_license.reload }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+
+ context 'when software_license has spdx_identifier' do
+ let(:software_license) { create(:software_license, :mit) }
+
+ it 'deletes software_license_policy only' do
+ service.execute(software_license_policy)
+
+ expect { software_license_policy.reload }.to raise_error(ActiveRecord::RecordNotFound)
+ expect { software_license.reload }.not_to raise_error
+ end
+ end
+
+ context 'when software_license has multiple software_license_policies' do
+ before do
+ create(:software_license_policy, software_license: software_license)
+ end
+
+ it 'deletes software_license_policy only' do
+ service.execute(software_license_policy)
+
+ expect { software_license_policy.reload }.to raise_error(ActiveRecord::RecordNotFound)
+ expect { software_license.reload }.not_to raise_error
+ end
+ end
+ end
+end
--
GitLab