Merge dependency location

Add dependency path to response

Fix failed cops

Use only blob_path for location

Merge dependencies locations to license report

Move this functionality to License Scanning report

Try to improve merge method

ee/app/serializers/license_entity.rb

@@ -3,6 +3,7 @@
 class LicenseEntity < Grape::Entity
   class ComponentEntity < Grape::Entity
     expose :name
+    expose :path, as: :blob_path
   end
 
   expose :name

ee/app/services/security/licenses_list_service.rb

@@ -8,11 +8,20 @@ module Security
     end
 
     def execute
-      pipeline.license_scanning_report.licenses
+      report.merge_dependencies_info!(dependencies) if dependencies.any?
+      report.licenses
     end
 
     private
 
     attr_reader :pipeline
+
+    def dependencies
+      @dependencies ||= pipeline.dependency_list_report.dependencies_with_licenses
+    end
+
+    def report
+      @report ||= pipeline.license_scanning_report
+    end
   end
 end

ee/lib/gitlab/ci/reports/dependency_list/report.rb

@@ -23,6 +23,10 @@ module Gitlab
               dependency[:licenses].push(name: license.name, url: license.url)
             end
           end
+
+          def dependencies_with_licenses
+            dependencies.select { |dependency| dependency[:licenses].any? }
+          end
         end
       end
     end

ee/lib/gitlab/ci/reports/license_scanning/dependency.rb

@@ -5,6 +5,7 @@ module Gitlab
     module Reports
       module LicenseScanning
         class Dependency
+          attr_accessor :path
           attr_reader :name
 
           def initialize(name)

ee/lib/gitlab/ci/reports/license_scanning/report.rb

@@ -33,6 +33,28 @@ module Gitlab
             found_licenses[license.canonical_id] ||= license
           end
 
+          def by_license_name(name)
+            licenses.find { |license| license.name == name }
+          end
+
+          def merge_dependencies_info!(dependencies_with_licenses)
+            return if found_licenses.empty?
+
+            found_licenses.values.each do |license|
+              matched_dependencies = dependencies_with_licenses.select do |dependency|
+                dependency[:licenses].map { |l| l[:name] }.include?(license.name)
+              end
+
+              matched_dependencies.each do |dependency|
+                license_dependency = license.dependencies.find { |l_dependency| l_dependency.name == dependency[:name] }
+
+                next unless license_dependency
+
+                license_dependency.path = dependency.dig(:location, :blob_path)
+              end
+            end
+          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)

ee/spec/factories/dependencies.rb

@@ -2,17 +2,21 @@
 
 FactoryBot.define do
   factory :dependency, class: Hash do
-    name { 'nokogiri' }
+    sequence(:name) { |n| "library#{n}" }
     packager { 'Ruby (Bundler)' }
     version { '1.8.0' }
     licenses { [] }
-    location do
+    sequence(:location) do |n|
       {
-        blob_path: '/some_project/path/Gemfile.lock',
-        path:      'Gemfile.lock'
+        blob_path: "/some_project/path/File_#{n}.lock",
+        path:      "File_#{n}.lock"
       }
     end
 
+    trait :nokogiri do
+      name { 'nokogiri' }
+    end
+
     trait :with_vulnerabilities do
       vulnerabilities do
         [{

ee/spec/fixtures/api/schemas/licenses_list.json

@@ -26,6 +26,9 @@
             "properties": {
               "name": {
                 "type": "string"
+              },
+              "blob_path": {
+                "type": "string"
               }
             }
           }

ee/spec/lib/gitlab/ci/parsers/security/dependency_list_spec.rb

@@ -55,7 +55,7 @@ describe Gitlab::Ci::Parsers::Security::DependencyList do
 
   describe '#parse_licenses!' do
     let(:artifact) { create(:ee_ci_job_artifact, :license_management) }
-    let(:dependency_info) { build(:dependency, :with_vulnerabilities) }
+    let(:dependency_info) { build(:dependency, :nokogiri, :with_vulnerabilities) }
 
     before do
       report.add_dependency(dependency)

ee/spec/lib/gitlab/ci/reports/dependency_list/report_spec.rb

@@ -57,4 +57,41 @@ describe Gitlab::Ci::Reports::DependencyList::Report do
       end
     end
   end
+
+  describe '#dependencies_with_licenses' do
+    subject { report.dependencies_with_licenses }
+
+    context 'with found dependencies' do
+      let(:plain_dependency) { build :dependency }
+
+      before do
+        report.add_dependency(plain_dependency)
+      end
+
+      context 'with existing license' do
+        let(:dependency) { build :dependency, :with_licenses }
+
+        before do
+          report.add_dependency(dependency)
+        end
+
+        it 'returns only dependency with license' do
+          expect(subject.size).to eq(1)
+          expect(subject.first).to eq(dependency)
+        end
+      end
+
+      context 'without existing license' do
+        it 'returns empty array' do
+          expect(subject).to be_empty
+        end
+      end
+    end
+
+    context 'without found dependencies' do
+      it 'returns empty array' do
+        expect(subject).to be_empty
+      end
+    end
+  end
 end

ee/spec/lib/gitlab/ci/reports/license_scanning/report_spec.rb

@@ -3,19 +3,92 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Reports::LicenseScanning::Report do
-  subject { build(:ci_reports_license_scanning_report, :mit) }
+  include LicenseScanningReportHelpers
+
+  describe '#by_license_name' do
+    subject { report.by_license_name(name) }
+
+    let(:report) { build(:ci_reports_license_scanning_report, :report_2) }
+
+    context 'with existing license' do
+      let(:name) { 'MIT' }
+
+      it 'finds right name' do
+        is_expected.to be_a(Gitlab::Ci::Reports::LicenseScanning::License)
+        expect(subject.name).to eq('MIT')
+      end
+    end
+
+    context 'without existing license' do
+      let(:name) { 'TIM' }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#merge_dependencies_info!' do
+    subject { report.merge_dependencies_info!(dependencies) }
+
+    let(:report) { build(:ci_reports_license_scanning_report, :report_2) }
+    let(:dependency_list_report) { Gitlab::Ci::Reports::DependencyList::Report.new }
+
+    context 'without licensed dependencies' do
+      let(:library1) { build(:dependency, name: 'Library1') }
+      let(:library3) { build(:dependency, name: 'Library3') }
+      let(:dependencies) { [library3, library1] }
+
+      before do
+        subject
+      end
+
+      it 'does not merge dependency path' do
+        paths = all_dependency_paths(report)
+
+        expect(paths).to be_empty
+      end
+    end
+
+    context 'with licensed dependencies' do
+      let(:library1) { build(:dependency, :with_licenses, name: 'Library1') }
+      let(:library3) { build(:dependency, :with_licenses, name: 'Library3') }
+      let(:library4) { build(:dependency, :with_licenses, name: 'Library4') }
+      let(:dependencies) { [library1, library3, library4] }
+
+      let(:mit_license) { report.by_license_name('MIT') }
+      let(:apache_license) { report.by_license_name('Apache 2.0') }
+
+      before do
+        mit_license.add_dependency('Library4')
+        apache_license.add_dependency('Library3')
+
+        subject
+      end
+
+      it 'merge path to matched dependencies' do
+        dep1 = dependency_by_name(mit_license, 'Library1')
+        dep4 = dependency_by_name(mit_license, 'Library4')
+        dep3 = dependency_by_name(apache_license, 'Library3')
+
+        expect(dep1.path).to eq(library1.dig(:location, :blob_path))
+        expect(dep4.path).to eq(library4.dig(:location, :blob_path))
+        expect(dep3.path).to be_nil
+      end
+    end
+  end
 
   describe '#violates?' do
+    subject { report.violates?(project.software_license_policies) }
+
     let(:project) { create(:project) }
 
     context "when checking for violations using v1 license scan report" do
-      subject { build(:license_scan_report) }
+      let(:report) { build(:license_scan_report) }
 
       let(:mit_license) { build(:software_license, :mit, spdx_identifier: nil) }
       let(:apache_license) { build(:software_license, :apache_2_0, spdx_identifier: nil) }
 
       before do
-        subject
+        report
           .add_license(id: nil, name: 'MIT')
           .add_dependency('rails')
       end
@@ -27,7 +100,7 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
           project.software_license_policies << mit_blacklist
         end
 
-        specify { expect(subject.violates?(project.software_license_policies)).to be(true) }
+        it { is_expected.to be_truthy }
       end
 
       context 'when a blacklisted license is discovered with a different casing for the name' do
@@ -38,7 +111,7 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
           project.software_license_policies << mit_blacklist
         end
 
-        specify { expect(subject.violates?(project.software_license_policies)).to be(true) }
+        it { is_expected.to be_truthy }
       end
 
       context 'when none of the licenses discovered in the report violate the blacklist policy' do
@@ -48,12 +121,12 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
           project.software_license_policies << apache_blacklist
         end
 
-        specify { expect(subject.violates?(project.software_license_policies)).to be(false) }
+        it { is_expected.to be_falsey }
       end
     end
 
     context "when checking for violations using the v2 license scan reports" do
-      subject { build(:license_scan_report) }
+      let(:report) { build(:license_scan_report) }
 
       context "when a blacklisted license with a SPDX identifier is also in the report" do
         let(:mit_spdx_id) { 'MIT' }
@@ -61,11 +134,11 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
         let(:mit_policy) { build(:software_license_policy, :blacklist, software_license: mit_license) }
 
         before do
-          subject.add_license(id: mit_spdx_id, name: 'MIT License')
+          report.add_license(id: mit_spdx_id, name: 'MIT License')
           project.software_license_policies << mit_policy
         end
 
-        specify { expect(subject.violates?(project.software_license_policies)).to be(true) }
+        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
@@ -73,11 +146,11 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
         let(:custom_policy) { build(:software_license_policy, :blacklist, software_license: custom_license) }
 
         before do
-          subject.add_license(id: nil, name: 'Custom')
+          report.add_license(id: nil, name: 'Custom')
           project.software_license_policies << custom_policy
         end
 
-        specify { expect(subject.violates?(project.software_license_policies)).to be(true) }
+        it { is_expected.to be_truthy }
       end
 
       context "when none of the licenses discovered match any of the blacklisted software policies" do
@@ -85,12 +158,12 @@ describe Gitlab::Ci::Reports::LicenseScanning::Report do
         let(:apache_policy) { build(:software_license_policy, :blacklist, software_license: apache_license) }
 
         before do
-          subject.add_license(id: nil, name: 'Custom')
-          subject.add_license(id: 'MIT', name: 'MIT License')
+          report.add_license(id: nil, name: 'Custom')
+          report.add_license(id: 'MIT', name: 'MIT License')
           project.software_license_policies << apache_policy
         end
 
-        specify { expect(subject.violates?(project.software_license_policies)).to be(false) }
+        it { is_expected.to be_falsey }
       end
     end
   end

ee/spec/models/ci/build_spec.rb

@@ -267,7 +267,7 @@ describe Ci::Build do
   describe '#collect_licenses_for_dependency_list!' do
     let!(:lm_artifact) { create(:ee_ci_job_artifact, :license_management, job: job, project: job.project) }
     let(:dependency_list_report) { Gitlab::Ci::Reports::DependencyList::Report.new }
-    let(:dependency) { build(:dependency) }
+    let(:dependency) { build(:dependency, :nokogiri) }
 
     subject { job.collect_licenses_for_dependency_list!(dependency_list_report) }
 

ee/spec/serializers/dependency_entity_spec.rb

@@ -32,24 +32,24 @@ describe DependencyEntity do
       end
 
       context 'with reporter' do
-        let(:dependency_info) { build(:dependency, :with_licenses) }
-
         before do
           project.add_reporter(user)
         end
 
-        it { is_expected.to eq(dependency_info) }
+        it 'includes license info and not vulnerabilities' do
+          is_expected.to eq(dependency.except(:vulnerabilities))
+        end
       end
     end
 
     context 'when all required features are unavailable' do
-      let(:dependency_info) { build(:dependency).except(:licenses) }
-
       before do
         project.add_developer(user)
       end
 
-      it { is_expected.to eq(dependency_info) }
+      it 'does not include licenses and vulnerabilities' do
+        is_expected.to eq(dependency.except(:vulnerabilities, :licenses))
+      end
     end
   end
 end

ee/spec/serializers/license_entity_spec.rb

@@ -12,10 +12,17 @@ describe LicenseEntity do
       {
         name:       'MIT',
         url:        'https://opensource.org/licenses/mit',
-        components: [{ name: 'rails' }]
+        components: [{
+                       name:     'rails',
+                       blob_path: 'some_path'
+                     }]
       }
     end
 
+    before do
+      license.dependencies.first.path = 'some_path'
+    end
+
     it { is_expected.to eq(assert_license) }
   end
 end

ee/spec/services/security/licenses_list_service_spec.rb

@@ -3,17 +3,43 @@
 require 'spec_helper'
 
 describe Security::LicensesListService do
-  describe '#execute' do
-    let!(:pipeline) { create(:ee_ci_pipeline, :with_license_management_report) }
+  include LicenseScanningReportHelpers
 
+  describe '#execute' do
     subject { described_class.new(pipeline: pipeline).execute }
 
+    let!(:pipeline) { create(:ee_ci_pipeline, :with_license_management_report) }
+    let(:project) { pipeline.project }
+    let(:mit_license) { find_license_by_name(subject, 'MIT') }
+
     before do
-      stub_licensed_features(license_management: true)
+      stub_licensed_features(license_management: true, dependency_list: true)
     end
 
-    it 'returns array of Licenses' do
-      is_expected.to be_an(Array)
+    context 'with matching dependency list' do
+      let!(:build) { create(:ci_build, :success, name: 'dependency_list', pipeline: pipeline, project: project) }
+      let!(:artifact) { create(:ee_ci_job_artifact, :dependency_list, job: build, project: project) }
+
+      it 'merges dependency location for found dependencies' do
+        nokogiri = dependency_by_name(mit_license, 'nokogiri')
+        actioncable = dependency_by_name(mit_license, 'actioncable')
+        nokogiri_path = "/#{project.full_path}/blob/#{pipeline.sha}/rails/Gemfile.lock"
+
+        expect(nokogiri.path).to eq(nokogiri_path)
+        expect(actioncable.path).to be_nil
+      end
+    end
+
+    context 'without matching dependency list' do
+      it 'returns array of Licenses' do
+        is_expected.to be_an(Array)
+      end
+
+      it 'returns empty path in dependencies' do
+        nokogiri = dependency_by_name(mit_license, 'nokogiri')
+
+        expect(nokogiri.path).to be_nil
+      end
     end
   end
 end

ee/spec/support/helpers/license_scanning_report_helpers.rb

+# frozen_string_literal: true
+
+module LicenseScanningReportHelpers
+  def all_dependency_paths(report)
+    report.licenses.map { |license| license.dependencies.map(&:path) }.flatten.compact
+  end
+
+  def dependency_by_name(license, name)
+    license.dependencies.find { |dep| dep.name == name }
+  end
+
+  def find_license_by_name(licenses, name)
+    licenses.find { |license| license.name == name }
+  end
+end