Commit fcea0e21 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch '38271-blacker-the-berry' into 'master'

Rename blacklisted to denied

See merge request gitlab-org/gitlab!21261
parents 74289860 655279d6
......@@ -5,6 +5,9 @@
# For use in the License Management feature.
class SoftwareLicensePolicy < ApplicationRecord
include Presentable
APPROVAL_STATUS = {
'blacklisted' => 'denied'
}.freeze
# Only allows modification of the approval status
FORM_EDITABLE = %i[approval_status].freeze
......@@ -14,7 +17,7 @@ class SoftwareLicensePolicy < ApplicationRecord
attr_readonly :software_license
enum classification: {
blacklisted: 0,
denied: 0,
approved: 1
}
......@@ -25,7 +28,7 @@ class SoftwareLicensePolicy < ApplicationRecord
validates_presence_of :project
validates :classification, presence: true
# A license is unique for its project since it can't be approved and blacklisted.
# A license is unique for its project since it can't be approved and denied.
validates :software_license, uniqueness: { scope: :project_id }
scope :ordered, -> { SoftwareLicensePolicy.includes(:software_license).order("software_licenses.name ASC") }
......@@ -44,7 +47,15 @@ class SoftwareLicensePolicy < ApplicationRecord
delegate :name, :spdx_identifier, to: :software_license
def approval_status
APPROVAL_STATUS.key(classification) || classification
end
def self.workaround_cache_key
pluck(:id, :classification).flatten
end
def self.to_classification(approval_status)
APPROVAL_STATUS.fetch(approval_status, approval_status)
end
end
......@@ -10,7 +10,7 @@ class LicenseScanningReportLicenseEntity < Grape::Entity
expose :url
def classification
default = { id: nil, name: value_for(:name), classification: 'unclassified' }
default = { id: nil, name: value_for(:name), approval_status: 'unclassified' }
found = SoftwareLicensePoliciesFinder.new(request&.current_user, request&.project, name: value_for(:name)).find
ManagedLicenseEntity.represent(found || default)
end
......
......@@ -2,7 +2,7 @@
class ManagedLicenseEntity < Grape::Entity
expose :id
expose :classification, as: :approval_status
expose :approval_status
expose :software_license, merge: true do
expose :name
end
......
......@@ -17,8 +17,8 @@ module Projects
private
def change_classification_of(policy)
if blacklisted_classification?
policy.blacklisted!
if denied_classification?
policy.denied!
else
policy.approved!
end
......@@ -40,8 +40,8 @@ module Projects
SoftwareLicensePolicy.classifications.key?(params[:classification])
end
def blacklisted_classification?
params[:classification] == 'blacklisted'
def denied_classification?
params[:classification] == 'denied'
end
end
end
......
......@@ -24,7 +24,7 @@ module SoftwareLicensePolicies
policy = SoftwareLicense.create_policy_for!(
project: project,
name: params[:name],
classification: params[:approval_status]
classification: SoftwareLicensePolicy.to_classification(params[:approval_status])
)
RefreshLicenseComplianceChecksWorker.perform_async(project.id)
policy
......
......@@ -11,14 +11,11 @@ module SoftwareLicensePolicies
return error("", 403) unless can?(@current_user, :admin_software_license_policy, @project)
return success(software_license_policy: software_license_policy) unless params[:approval_status].present?
begin
software_license_policy.update(classification: params[:approval_status])
software_license_policy.update(classification: SoftwareLicensePolicy.to_classification(params[:approval_status]))
RefreshLicenseComplianceChecksWorker.perform_async(project.id)
rescue ArgumentError => ex
return error(ex.message, 400)
end
success(software_license_policy: software_license_policy)
rescue ArgumentError => ex
error(ex.message, 400)
end
end
end
......@@ -861,7 +861,7 @@ module EE
class ManagedLicense < Grape::Entity
expose :id, :name
expose :classification, as: :approval_status
expose :approval_status
end
class ProjectAlias < Grape::Entity
......
......@@ -63,8 +63,8 @@ module Gitlab
end
def violates?(software_license_policies)
policies_with_matching_license_name = software_license_policies.blacklisted.with_license_by_name(license_names)
policies_with_matching_spdx_id = software_license_policies.blacklisted.by_spdx(licenses.map(&:id).compact)
policies_with_matching_license_name = software_license_policies.denied.with_license_by_name(license_names)
policies_with_matching_spdx_id = software_license_policies.denied.by_spdx(licenses.map(&:id).compact)
policies_with_matching_spdx_id.or(policies_with_matching_license_name).exists?
end
......
......@@ -100,7 +100,7 @@ describe Projects::Security::LicensesController do
"spdx_identifier" => "MIT",
"name" => mit.name,
"url" => "http://spdx.org/licenses/MIT.json",
"classification" => "blacklisted"
"classification" => "denied"
})
expect(json_response.dig("licenses", 2)).to include({
......@@ -202,7 +202,7 @@ describe Projects::Security::LicensesController do
post :create, xhr: true, params: default_params.merge({
software_license_policy: {
software_license_id: mit_license.id,
classification: 'blacklisted'
classification: 'denied'
}
})
end
......@@ -210,14 +210,14 @@ describe Projects::Security::LicensesController do
it { expect(response).to have_http_status(:created) }
it 'creates a new policy' do
expect(project.reload.software_license_policies.blacklisted.count).to be(1)
expect(project.reload.software_license_policies.blacklisted.last.software_license).to eq(mit_license)
expect(project.reload.software_license_policies.denied.count).to be(1)
expect(project.reload.software_license_policies.denied.last.software_license).to eq(mit_license)
end
it 'returns the proper JSON response' do
expect(json[:id]).to be_present
expect(json[:spdx_identifier]).to eq(mit_license.spdx_identifier)
expect(json[:classification]).to eq('blacklisted')
expect(json[:classification]).to eq('denied')
expect(json[:name]).to eq(mit_license.name)
expect(json[:url]).to be_nil
expect(json[:components]).to be_empty
......@@ -319,18 +319,18 @@ describe Projects::Security::LicensesController do
before do
patch :update, xhr: true, params: default_params.merge({
software_license_policy: {
classification: "blacklisted"
classification: "denied"
}
})
end
it { expect(response).to have_http_status(:ok) }
it { expect(software_license_policy.reload).to be_blacklisted }
it { expect(software_license_policy.reload).to be_denied }
it "generates the proper JSON response" do
expect(json[:id]).to eql(software_license_policy.id)
expect(json[:spdx_identifier]).to eq(mit_license.spdx_identifier)
expect(json[:classification]).to eq("blacklisted")
expect(json[:classification]).to eq("denied")
expect(json[:name]).to eq(mit_license.name)
end
end
......
......@@ -6,16 +6,12 @@ FactoryBot.define do
project
software_license
trait :blacklist do
classification { :blacklisted }
end
trait :allowed do
classification { :approved }
end
trait :denied do
classification { :blacklisted }
classification { :denied }
end
end
end
......@@ -93,32 +93,32 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
.add_dependency('rails')
end
context 'when a blacklisted license is found in the report' do
let(:mit_blacklist) { build(:software_license_policy, :blacklist, software_license: mit_license) }
context 'when a denied license is found in the report' do
let(:denied_policy) { build(:software_license_policy, :denied, software_license: mit_license) }
before do
project.software_license_policies << mit_blacklist
project.software_license_policies << denied_policy
end
it { is_expected.to be_truthy }
end
context 'when a blacklisted license is discovered with a different casing for the name' do
let(:mit_blacklist) { build(:software_license_policy, :blacklist, software_license: mit_license) }
context 'when a denied license is discovered with a different casing for the name' do
let(:denied_policy) { build(:software_license_policy, :denied, software_license: mit_license) }
before do
mit_license.update!(name: 'mit')
project.software_license_policies << mit_blacklist
project.software_license_policies << denied_policy
end
it { is_expected.to be_truthy }
end
context 'when none of the licenses discovered in the report violate the blacklist policy' do
let(:apache_blacklist) { build(:software_license_policy, :blacklist, software_license: apache_license) }
context 'when none of the licenses discovered in the report violate the denied policy' do
let(:denied_policy) { build(:software_license_policy, :denied, software_license: apache_license) }
before do
project.software_license_policies << apache_blacklist
project.software_license_policies << denied_policy
end
it { is_expected.to be_falsey }
......@@ -128,10 +128,10 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
context "when checking for violations using the v2 license scan reports" do
let(:report) { build(:license_scan_report) }
context "when a blacklisted license with a SPDX identifier is also in the report" do
context "when a denied license with a SPDX identifier is also in the report" do
let(:mit_spdx_id) { 'MIT' }
let(:mit_license) { build(:software_license, :mit, spdx_identifier: mit_spdx_id) }
let(:mit_policy) { build(:software_license_policy, :blacklist, software_license: mit_license) }
let(:mit_policy) { build(:software_license_policy, :denied, software_license: mit_license) }
before do
report.add_license(id: mit_spdx_id, name: 'MIT License')
......@@ -141,9 +141,9 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
it { is_expected.to be_truthy }
end
context "when a blacklisted license does not have an SPDX identifier because it was provided by an end user" do
context "when a denied license does not have an SPDX identifier because it was provided by an end user" do
let(:custom_license) { build(:software_license, name: 'custom', spdx_identifier: nil) }
let(:custom_policy) { build(:software_license_policy, :blacklist, software_license: custom_license) }
let(:custom_policy) { build(:software_license_policy, :denied, software_license: custom_license) }
before do
report.add_license(id: nil, name: 'Custom')
......@@ -153,9 +153,9 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
it { is_expected.to be_truthy }
end
context "when none of the licenses discovered match any of the blacklisted software policies" do
context "when none of the licenses discovered match any of the denied software policies" do
let(:apache_license) { build(:software_license, :apache_2_0, spdx_identifier: 'Apache-2.0') }
let(:apache_policy) { build(:software_license_policy, :blacklist, software_license: apache_license) }
let(:apache_policy) { build(:software_license_policy, :denied, software_license: apache_license) }
before do
report.add_license(id: nil, name: 'Custom')
......
......@@ -336,7 +336,7 @@ describe ApprovalMergeRequestRule do
let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_management, project: project) }
let(:project) { create(:project) }
let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
let!(:blacklist_policy) { create(:software_license_policy, project: project, software_license: license, classification: :blacklisted) }
let!(:denied_policy) { create(:software_license_policy, project: project, software_license: license, classification: :denied) }
before do
subject.refresh_required_approvals!(project_approval_rule)
......
......@@ -24,7 +24,7 @@ RSpec.describe SCA::LicenseCompliance do
expect(subject.policies[0].id).to eq(mit_policy.id)
expect(subject.policies[0].name).to eq(mit.name)
expect(subject.policies[0].url).to be_nil
expect(subject.policies[0].classification).to eq("blacklisted")
expect(subject.policies[0].classification).to eq("denied")
expect(subject.policies[0].spdx_identifier).to eq("MIT")
end
end
......@@ -102,7 +102,7 @@ RSpec.describe SCA::LicenseCompliance do
expect(subject.policies[1].id).to eq(mit_policy.id)
expect(subject.policies[1].name).to eq(mit.name)
expect(subject.policies[1].url).to eq("http://spdx.org/licenses/MIT.json")
expect(subject.policies[1].classification).to eq("blacklisted")
expect(subject.policies[1].classification).to eq("denied")
expect(subject.policies[1].spdx_identifier).to eq("MIT")
expect(subject.policies[2].id).to eq(other_license_policy.id)
......@@ -145,7 +145,7 @@ RSpec.describe SCA::LicenseCompliance do
expect(subject.policies[1].id).to eq(mit_policy.id)
expect(subject.policies[1].name).to eq(mit.name)
expect(subject.policies[1].url).to eq("http://opensource.org/licenses/mit-license")
expect(subject.policies[1].classification).to eq("blacklisted")
expect(subject.policies[1].classification).to eq("denied")
expect(subject.policies[1].spdx_identifier).to eq("MIT")
expect(subject.policies[2].id).to eq(other_license_policy.id)
......
......@@ -80,7 +80,7 @@ RSpec.describe SCA::LicensePolicy do
context "when a denied software_policy is provided" do
let(:policy) { build(:software_license_policy, :denied, software_license: software_license) }
it { expect(subject.classification).to eq("blacklisted") }
it { expect(subject.classification).to eq("denied") }
end
context "when a software_policy is NOT provided" do
......
......@@ -28,10 +28,10 @@ describe SoftwareLicense do
context 'when a software license with a given name has NOT been created' do
let(:license_name) { SecureRandom.uuid }
let(:result) { subject.create_policy_for!(project: project, name: license_name, classification: :blacklisted) }
let(:result) { subject.create_policy_for!(project: project, name: license_name, classification: :denied) }
specify { expect(result).to be_persisted }
specify { expect(result).to be_blacklisted }
specify { expect(result).to be_denied }
specify { expect(result.software_license).to be_persisted }
specify { expect(result.software_license.name).to eql(license_name) }
end
......
......@@ -38,7 +38,7 @@ describe API::ManagedLicenses do
expect(json_response).to be_a(Array)
expect(json_response.first['id']).to eq(software_license_policy.id)
expect(json_response.first['name']).to eq(software_license_policy.name)
expect(json_response.first['approval_status']).to eq(software_license_policy.classification)
expect(json_response.first['approval_status']).to eq('approved')
end
end
......@@ -51,7 +51,7 @@ describe API::ManagedLicenses do
expect(json_response).to be_a(Array)
expect(json_response.first['id']).to eq(software_license_policy.id)
expect(json_response.first['name']).to eq(software_license_policy.name)
expect(json_response.first['approval_status']).to eq(software_license_policy.classification)
expect(json_response.first['approval_status']).to eq('approved')
end
end
......@@ -92,7 +92,7 @@ describe API::ManagedLicenses do
expect(response).to match_response_schema('software_license_policy', dir: 'ee')
expect(json_response['id']).to eq(software_license_policy.id)
expect(json_response['name']).to eq(software_license_policy.name)
expect(json_response['approval_status']).to eq(software_license_policy.classification)
expect(json_response['approval_status']).to eq('approved')
end
it 'returns project managed license details using the license name as key' do
......@@ -103,7 +103,7 @@ describe API::ManagedLicenses do
expect(response).to match_response_schema('software_license_policy', dir: 'ee')
expect(json_response['id']).to eq(software_license_policy.id)
expect(json_response['name']).to eq(software_license_policy.name)
expect(json_response['approval_status']).to eq(software_license_policy.classification)
expect(json_response['approval_status']).to eq('approved')
end
it 'responds with 404 Not Found if requesting non-existing managed license' do
......@@ -121,7 +121,7 @@ describe API::ManagedLicenses do
expect(response).to match_response_schema('software_license_policy', dir: 'ee')
expect(json_response['id']).to eq(software_license_policy.id)
expect(json_response['name']).to eq(software_license_policy.name)
expect(json_response['approval_status']).to eq(software_license_policy.classification)
expect(json_response['approval_status']).to eq('approved')
end
end
......@@ -220,10 +220,10 @@ describe API::ManagedLicenses do
# Check that response is equal to the updated object
expect(json_response['id']).to eq(initial_id)
expect(json_response['name']).to eq(updated_software_license_policy.name)
expect(json_response['approval_status']).to eq(updated_software_license_policy.classification)
expect(json_response['approval_status']).to eq('blacklisted')
# Check that the approval status was updated
expect(updated_software_license_policy.classification).to eq('blacklisted')
expect(updated_software_license_policy).to be_denied
# Check that response is equal to the old object except for the approval status
expect(initial_id).to eq(updated_software_license_policy.id)
......
......@@ -19,6 +19,10 @@ describe LicenseScanningReportLicenseEntity do
describe '#as_json' do
subject { entity.as_json }
it 'emits the correct approval_status' do
expect(subject[:classification][:approval_status]).to eq('unclassified')
end
it 'contains the correct dependencies' do
expect(subject[:dependencies].count).to eq(2)
expect(subject[:dependencies][0][:name]).to eq('Dependency1')
......
......@@ -12,5 +12,20 @@ describe ManagedLicenseEntity do
it 'contains required fields' do
expect(subject).to include(:id, :name, :approval_status)
end
describe "#approval_status" do
where(:classification, :approval_status) do
[
%w[approved approved],
%w[denied blacklisted]
]
end
with_them do
let(:software_license_policy) { build(:software_license_policy, classification: classification) }
it { expect(subject[:approval_status]).to eql(approval_status) }
end
end
end
end
......@@ -42,7 +42,7 @@ describe Projects::Licenses::CreatePolicyService do
let(:params) do
{
spdx_identifier: mit_license.spdx_identifier,
classification: 'blacklisted'
classification: 'denied'
}
end
......@@ -53,7 +53,7 @@ describe Projects::Licenses::CreatePolicyService do
expect(result[:software_license_policy]).to be_present
expect(result[:software_license_policy].id).to be_present
expect(result[:software_license_policy].spdx_identifier).to eq(mit_license.spdx_identifier)
expect(result[:software_license_policy].classification).to eq('blacklisted')
expect(result[:software_license_policy].classification).to eq('denied')
expect(result[:software_license_policy].name).to eq(mit_license.name)
expect(result[:software_license_policy].url).to be_nil
expect(result[:software_license_policy].dependencies).to be_empty
......@@ -65,7 +65,7 @@ describe Projects::Licenses::CreatePolicyService do
let(:params) do
{
spdx_identifier: nil,
classification: 'blacklisted'
classification: 'denied'
}
end
......
......@@ -62,12 +62,12 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
end
context "license compliance policy" do
let!(:software_license_policy) { create(:software_license_policy, :blacklist, project: project, software_license: blacklisted_license) }
let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) }
let!(:blacklisted_license) { create(:software_license) }
let!(:denied_license) { create(:software_license) }
context "when a license violates the license compliance policy" do
let!(:blacklisted_license) { create(:software_license, name: license_name) }
let!(:denied_license) { create(:software_license, name: license_name) }
let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
let!(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] }
......@@ -139,9 +139,9 @@ describe Security::SyncReportsToApprovalRulesService, '#execute' do
end
context "license compliance policy" do
let!(:software_license_policy) { create(:software_license_policy, :blacklist, project: project, software_license: blacklisted_license) }
let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
let!(:license_compliance_rule) { create(:report_approver_rule, :license_management, merge_request: merge_request, approvals_required: 1) }
let!(:blacklisted_license) { create(:software_license) }
let!(:denied_license) { create(:software_license) }
specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
specify { expect(subject[:status]).to be(:success) }
......
......@@ -30,16 +30,17 @@ describe SoftwareLicensePolicies::CreateService do
end
context 'with a user who is allowed to admin' do
it 'creates one software license policy correctly' do
expect { subject.execute }.to change { project.software_license_policies.count }.from(0).to(1)
software_license_policy = project.software_license_policies.last
expect(software_license_policy).to be_persisted
expect(software_license_policy.name).to eq(params[:name])
expect(software_license_policy.classification).to eq('blacklisted')
context 'when valid parameters are specified' do
where(:approval_status, :expected_classification) do
[
%w[approved approved],
%w[denied denied],
%w[blacklisted denied]
]
end
context "when valid parameters are specified" do
with_them do
let(:params) { { name: 'MIT', approval_status: approval_status } }
let(:result) { subject.execute }
before do
......@@ -47,12 +48,16 @@ describe SoftwareLicensePolicies::CreateService do
result
end
specify { expect(result[:status]).to be(:success) }
specify { expect(result[:software_license_policy]).to be_present }
specify { expect(result[:software_license_policy]).to be_persisted }
specify { expect(result[:software_license_policy].name).to eql(params[:name]) }
specify { expect(result[:software_license_policy].classification).to eql('blacklisted') }
specify { expect(RefreshLicenseComplianceChecksWorker).to have_received(:perform_async).with(project.id) }
it 'creates one software license policy correctly' do
expect(project.software_license_policies.count).to be(1)
expect(result[:status]).to be(:success)
expect(result[:software_license_policy]).to be_present
expect(result[:software_license_policy]).to be_persisted
expect(result[:software_license_policy].name).to eql(params[:name])
expect(result[:software_license_policy].classification).to eql(expected_classification)
expect(RefreshLicenseComplianceChecksWorker).to have_received(:perform_async).with(project.id)
end
end
end
context "when an argument error is raised" do
......
......@@ -11,13 +11,7 @@ describe SoftwareLicensePolicies::UpdateService do
end
end
let(:software_license_policy) do
create(
:software_license_policy,
software_license: create(:software_license, name: 'ExamplePL/2.1'),
classification: 'blacklisted'
)
end
let(:software_license_policy) { create(:software_license_policy, :denied) }
before do
stub_licensed_features(license_management: true)
......
......@@ -21,10 +21,10 @@ describe RefreshLicenseComplianceChecksWorker do
let!(:closed_merge_request_approval_rule) { create(:report_approver_rule, :license_management, merge_request: closed_merge_request, approvals_required: 0) }
let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_management, project: project) }
context "when a license is blacklisted, that appears in some of the license management reports" do
context "when a license is denied, that appears in some of the license management reports" do
let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
let!(:blacklist_policy) { create(:software_license_policy, project: project, software_license: license, classification: :blacklisted) }
let!(:denied_policy) { create(:software_license_policy, :denied, project: project, software_license: license) }
let(:license) { create(:software_license, name: license_report.license_names[0]) }
let(:license_report) { open_pipeline.license_scanning_report }
......@@ -36,10 +36,10 @@ describe RefreshLicenseComplianceChecksWorker do
specify { expect(closed_merge_request_approval_rule.reload.approvals_required).to be_zero }
end
context "when none of the blacklisted licenses appear in the most recent license management reports" do
context "when none of the denied licenses appear in the most recent license management reports" do
let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
let!(:blacklist_policy) { create(:software_license_policy, project: project, software_license: license, classification: :blacklisted) }
let!(:denied_policy) { create(:software_license_policy, :denied, project: project, software_license: license) }
let(:license) { create(:software_license, name: SecureRandom.uuid) }
before do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment