From cf0a68b5c968e34808165be18bff912f097c98b9 Mon Sep 17 00:00:00 2001 From: atiwari71 Date: Thu, 10 Aug 2023 12:01:34 +0530 Subject: [PATCH 1/3] Adds Export Sbom APIs Adds POST /pipelines/:id/dependency_list_exports Changelog: added EE: true --- app/policies/ci/pipeline_policy.rb | 8 + .../dependencies/dependency_list_export.rb | 2 +- .../dependencies/create_export_service.rb | 11 +- .../sbom/pipeline_sbom_service.rb | 43 +++++ .../services/dependencies/export_service.rb | 43 ++++- .../development/merge_sbom_api.yml | 8 + ee/lib/api/dependency_list_exports.rb | 22 +++ ee/lib/ee/api/helpers.rb | 13 ++ .../dependencies/dependency_list_export.rb | 2 +- ee/spec/lib/ee/api/helpers_spec.rb | 41 +++++ .../api/dependency_list_exports_spec.rb | 36 +++- .../sbom/pipeline_sbom_service_spec.rb | 67 ++++++++ .../dependencies/export_service_spec.rb | 74 +++++++- lib/api/helpers.rb | 24 +++ spec/lib/api/helpers_spec.rb | 159 ++++++++++++++++++ spec/policies/ci/pipeline_policy_spec.rb | 25 +++ 16 files changed, 557 insertions(+), 21 deletions(-) create mode 100644 ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb create mode 100644 ee/config/feature_flags/development/merge_sbom_api.yml create mode 100644 ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 4d21da0226b6b5..d3fd059658d7a7 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -18,6 +18,10 @@ class PipelinePolicy < BasePolicy @subject.triggered_by?(@user) end + condition(:project_allows_read_dependencies) do + can?(:read_dependencies, @subject.project) + end + # Disallow users without permissions from accessing internal pipelines rule { ~can?(:read_build) & ~external_pipeline }.policy do prevent :read_pipeline @@ -41,6 +45,10 @@ class PipelinePolicy < BasePolicy enable :read_pipeline_variable end + rule { project_allows_read_dependencies }.policy do + enable :read_dependencies + end + def ref_protected?(user, project, tag, ref) access = ::Gitlab::UserAccess.new(user, container: project) diff --git a/ee/app/models/dependencies/dependency_list_export.rb b/ee/app/models/dependencies/dependency_list_export.rb index 5e5d6f6cb50099..a45a7763bd3063 100644 --- a/ee/app/models/dependencies/dependency_list_export.rb +++ b/ee/app/models/dependencies/dependency_list_export.rb @@ -17,7 +17,7 @@ class DependencyListExport < ApplicationRecord validate :only_one_exportable enum export_type: { - json: 0, + dependency_list: 0, sbom: 1 } diff --git a/ee/app/services/dependencies/create_export_service.rb b/ee/app/services/dependencies/create_export_service.rb index 2b976dbdaa89be..5af01d40d6b4dd 100644 --- a/ee/app/services/dependencies/create_export_service.rb +++ b/ee/app/services/dependencies/create_export_service.rb @@ -2,15 +2,20 @@ module Dependencies class CreateExportService - attr_reader :author, :exportable + attr_reader :author, :exportable, :export_type - def initialize(exportable, author) + def initialize(exportable, author, export_type = 'dependency_list') @author = author @exportable = exportable + @export_type = export_type end def execute - dependency_list_export = Dependencies::DependencyListExport.create!(exportable: exportable, author: author) + dependency_list_export = Dependencies::DependencyListExport.create!( + exportable: exportable, + author: author, + export_type: export_type + ) Dependencies::ExportWorker.perform_async(dependency_list_export.id) dependency_list_export rescue StandardError diff --git a/ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb b/ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb new file mode 100644 index 00000000000000..5004b6ecb3d135 --- /dev/null +++ b/ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Dependencies + module ExportSerializers + module Sbom + class PipelineSbomService + SchemaValidationError = Class.new(StandardError) + + def self.execute(dependency_list_export) + new(dependency_list_export).execute + end + + def initialize(dependency_list_export) + @dependency_list_export = dependency_list_export + + @pipeline = dependency_list_export.pipeline + @project = pipeline.project + end + + def execute + entity = serializer_service.execute + return entity if serializer_service.valid? + + raise SchemaValidationError, "Invalid CycloneDX report: #{serializer_service.errors.join(', ')}" + end + + private + + def serializer_service + @service ||= ::Sbom::ExportSerializers::JsonService.new(merged_report) + end + + def merged_report + ::Sbom::MergeReportsService.new( + pipeline.sbom_reports.reports, project + ).execute + end + + attr_reader :dependency_list_export, :scanner, :pipeline, :project + end + end + end +end diff --git a/ee/app/services/dependencies/export_service.rb b/ee/app/services/dependencies/export_service.rb index 867ef92079b848..793227c5306166 100644 --- a/ee/app/services/dependencies/export_service.rb +++ b/ee/app/services/dependencies/export_service.rb @@ -3,9 +3,14 @@ module Dependencies class ExportService SERIALIZER_SERVICES = { - Project => ExportSerializers::ProjectDependenciesService, - Group => ExportSerializers::GroupDependenciesService - }.freeze + dependency_list: { + Project => ExportSerializers::ProjectDependenciesService, + Group => ExportSerializers::GroupDependenciesService + }, + sbom: { + Ci::Pipeline => ExportSerializers::Sbom::PipelineSbomService + } + }.with_indifferent_access.freeze def self.execute(dependency_list_export) new(dependency_list_export).execute @@ -34,7 +39,7 @@ def create_export create_export_file dependency_list_export.finish! - rescue StandardError + rescue StandardError, Dependencies::ExportSerializers::Sbom::PipelineSbomService::SchemaValidationError dependency_list_export.reset_state! raise @@ -50,18 +55,42 @@ def create_export_file end def file_content - ::Gitlab::Json.dump(dependencies) + ::Gitlab::Json.dump(exported_object) end - def dependencies + def exported_object serializer_service.execute(dependency_list_export) end def serializer_service - SERIALIZER_SERVICES.fetch(exportable.class) + SERIALIZER_SERVICES.fetch(dependency_list_export.export_type).fetch(exportable.class) end def filename + if dependency_list_export.export_type == 'sbom' + sbom_filename + else + dependency_list_filename + end + end + + def sbom_filename + # Assuming dependency_list_export.export_type is sbom + # as we don't support dependency_list export_type for pipeline yet. + [ + 'gl-', + 'pipeline-', + exportable.id, + '-merged-', + Time.current.utc.strftime('%FT%H%M'), + '-sbom.', + 'cdx', + '.', + 'json' + ].join + end + + def dependency_list_filename [ exportable.full_path.parameterize, '_dependencies_', diff --git a/ee/config/feature_flags/development/merge_sbom_api.yml b/ee/config/feature_flags/development/merge_sbom_api.yml new file mode 100644 index 00000000000000..bcf6ecb08417ac --- /dev/null +++ b/ee/config/feature_flags/development/merge_sbom_api.yml @@ -0,0 +1,8 @@ +--- +name: merge_sbom_api +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/421513 +rollout_issue_url: +milestone: '16.4' +type: development +group: group::composition analysis +default_enabled: false diff --git a/ee/lib/api/dependency_list_exports.rb b/ee/lib/api/dependency_list_exports.rb index 10ce00d39152da..6317b916c347d4 100644 --- a/ee/lib/api/dependency_list_exports.rb +++ b/ee/lib/api/dependency_list_exports.rb @@ -39,6 +39,28 @@ class DependencyListExports < ::API::Base end end + resource :pipelines do + params do + requires :id, types: [String, Integer], desc: 'The ID of the pipeline' + + optional :export_type, type: String, values: %w[sbom dependency_list], desc: 'The type of the export file' + end + desc 'Generate a dependency list export on a pipeline-level' + post ':id/dependency_list_exports' do + not_found! unless Feature.enabled?(:merge_sbom_api, user_pipeline) + + # Currently, we only support sbom export type for this endpoint. + not_found! if params[:export_type] != 'sbom' + + authorize! :read_dependency, user_pipeline + + dependency_list_export = ::Dependencies::CreateExportService.new( + user_pipeline, current_user, params[:export_type]).execute + + present dependency_list_export, with: EE::API::Entities::DependencyListExport + end + end + params do requires :export_id, types: [Integer, String], desc: 'The ID of the dependency list export' end diff --git a/ee/lib/ee/api/helpers.rb b/ee/lib/ee/api/helpers.rb index 62241a56834ab8..24054390b24a35 100644 --- a/ee/lib/ee/api/helpers.rb +++ b/ee/lib/ee/api/helpers.rb @@ -88,6 +88,15 @@ def find_group!(id) end end + override :find_pipeline! + def find_pipeline!(id) + if job_token_authentication? + not_found!('Pipeline') + else + super + end + end + # rubocop: disable CodeReuse/ActiveRecord def find_group_epic(iid) EpicsFinder.new(current_user, group_id: user_group.id).find_by!(iid: iid) @@ -111,6 +120,10 @@ def find_subscription_add_on_purchase!(namespace, add_on) end # rubocop: enable CodeReuse/ActiveRecord + def user_pipeline + @pipeline ||= find_pipeline!(params[:id]) + end + private override :project_finder_params_ee diff --git a/ee/spec/factories/dependencies/dependency_list_export.rb b/ee/spec/factories/dependencies/dependency_list_export.rb index efc6f44c2dd787..bb3ad15aba20cd 100644 --- a/ee/spec/factories/dependencies/dependency_list_export.rb +++ b/ee/spec/factories/dependencies/dependency_list_export.rb @@ -5,7 +5,7 @@ project author status { 0 } - export_type { :json } + export_type { :dependency_list } trait :with_file do file { fixture_file_upload('ee/spec/fixtures/dependencies/dependencies.json') } diff --git a/ee/spec/lib/ee/api/helpers_spec.rb b/ee/spec/lib/ee/api/helpers_spec.rb index 3704a02a11da92..f553fe824cd853 100644 --- a/ee/spec/lib/ee/api/helpers_spec.rb +++ b/ee/spec/lib/ee/api/helpers_spec.rb @@ -131,6 +131,47 @@ def app end end + describe '#find_pipeline!' do + let(:pipeline) { create(:ci_pipeline) } + + let(:helper) do + stub_const('API::MockHelpers', Module.new) + + API::MockHelpers.module_eval do + def find_pipeline!(id) + Ci::Pipeline.find(id) + end + end + klass = Class.new do + include API::MockHelpers + include EE::API::Helpers + end + + klass.new + end + + subject(:find_pipeline!) { helper.find_pipeline!(pipeline.id) } + + context 'when authenticated using job token' do + before do + allow(helper).to receive(:job_token_authentication?).and_return(true) + allow(helper).to receive(:not_found!).and_raise(ActiveRecord::RecordNotFound) + end + + it 'returns not found' do + expect { find_pipeline! }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when not authenticated using job token' do + before do + allow(helper).to receive(:job_token_authentication?).and_return(false) + end + + it { is_expected.to eq pipeline } + end + end + describe '#find_subscription_add_on!' do subject(:find_or_create_subscription_add_on!) { helper.find_or_create_subscription_add_on!(name) } diff --git a/ee/spec/requests/api/dependency_list_exports_spec.rb b/ee/spec/requests/api/dependency_list_exports_spec.rb index db7eddb757a4d1..4c29721b8ec340 100644 --- a/ee/spec/requests/api/dependency_list_exports_spec.rb +++ b/ee/spec/requests/api/dependency_list_exports_spec.rb @@ -6,9 +6,12 @@ let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let(:export_type) { nil } + let(:params) { export_type } shared_examples_for 'creating dependency list export' do - subject(:create_dependency_list_export) { post api(request_path, user) } + subject(:create_dependency_list_export) { post api(request_path, user), params: params } context 'with user without permission' do before do @@ -50,8 +53,14 @@ stub_licensed_features(dependency_scanning: true, security_dashboard: true) end + let(:args) do + args = [exportable, user] + args << params[:export_type] if Hash(params)[:export_type] + args + end + it 'creates and returns a dependency_list_export' do - expect(::Dependencies::CreateExportService).to receive(:new).with(resource, user).and_call_original + expect(::Dependencies::CreateExportService).to receive(:new).with(*args).and_call_original create_dependency_list_export @@ -68,6 +77,7 @@ describe 'POST /projects/:id/dependency_list_exports' do let(:request_path) { "/projects/#{project.id}/dependency_list_exports" } let(:resource) { project } + let(:exportable) { project } it_behaves_like 'creating dependency list export' end @@ -75,6 +85,7 @@ describe 'POST /groups/:id/dependency_list_exports' do let(:request_path) { "/groups/#{group.id}/dependency_list_exports" } let(:resource) { group } + let(:exportable) { group } it_behaves_like 'creating dependency list export' @@ -91,6 +102,27 @@ end end + describe 'POST /pipelines/:id/dependency_list_exports' do + let(:request_path) { "/pipelines/#{pipeline.id}/dependency_list_exports" } + let(:resource) { project } + let(:exportable) { pipeline } + let(:params) { { export_type: 'sbom' } } + + it_behaves_like 'creating dependency list export' + + context 'when the `merge_sbom_api` feature flag is disabled' do + before do + stub_feature_flags(merge_sbom_api: false) + end + + it 'returns 404' do + post api(request_path, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET /dependency_list_exports/:export_id' do let(:dependency_list_export) { create(:dependency_list_export, :finished, author: user, project: project) } let(:request_path) { "/dependency_list_exports/#{dependency_list_export.id}" } diff --git a/ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb b/ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb new file mode 100644 index 00000000000000..31f575fdba8eb1 --- /dev/null +++ b/ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Dependencies::ExportSerializers::Sbom::PipelineSbomService, feature_category: :dependency_management do + let(:pipeline) { create(:ee_ci_pipeline, :with_cyclonedx_report) } + + describe '.execute' do + let(:dependency_list_export) { instance_double(Dependencies::DependencyListExport, pipeline: pipeline) } + + subject(:execute) { described_class.execute(dependency_list_export) } + + it 'instantiates a service object and sends execute message to it' do + expect_next_instance_of(described_class, dependency_list_export) do |service_object| + expect(service_object).to receive(:execute) + end + + execute + end + end + + describe '#execute' do + let(:dependency_list_export) { create(:dependency_list_export, exportable: pipeline) } + + let(:service_class) { described_class.new(dependency_list_export) } + + let(:components) { service_class.execute.as_json[:components] } + + before do + stub_licensed_features(dependency_scanning: true) + end + + context 'when the pipeline does not have cyclonedx reports' do + subject { components } + + let_it_be(:pipeline) { create(:ee_ci_pipeline) } + + it { is_expected.to be_empty } + end + + context 'when the pipeline has cyclonedx reports' do + it 'returns all the components' do + expect(components.count).to be 441 + end + end + + context 'when the merged report is not valid' do + let(:invalid_report) do + report = ::Gitlab::Ci::Reports::Sbom::Report.new + report.sbom_attributes = { invalid: 'json' } + report + end + + before do + allow_next_instance_of(::Sbom::MergeReportsService) do |service| + allow(service).to receive(:execute).and_return(invalid_report) + end + end + + it 'raises a SchemaValidationError' do + expect { service_class.execute }.to raise_error( + described_class::SchemaValidationError + ).with_message(/Invalid CycloneDX report: /) + end + end + end +end diff --git a/ee/spec/services/dependencies/export_service_spec.rb b/ee/spec/services/dependencies/export_service_spec.rb index 936221ea9afee2..d7f7e57cfa87f5 100644 --- a/ee/spec/services/dependencies/export_service_spec.rb +++ b/ee/spec/services/dependencies/export_service_spec.rb @@ -23,6 +23,10 @@ let(:finished_status) { 2 } let(:service_class) { described_class.new(dependency_list_export) } + before do + allow(Time).to receive(:current).and_return(Time.new(2023, 11, 14, 0, 0, 0, '+00:00')) + end + shared_examples_for 'export service' do |serializer_service| subject(:export) { service_class.execute } @@ -65,6 +69,7 @@ it 'attaches the file to export' do expect { export }.to change { dependency_list_export.file.read }.from(nil).to('"Foo"') + expect(dependency_list_export.file.filename).to eq(expected_filename) end it 'schedules the export deletion job' do @@ -77,17 +82,72 @@ end end - context 'when the exportable is a project' do - it_behaves_like 'export service', Dependencies::ExportSerializers::ProjectDependenciesService do - let(:dependency_list_export) { create(:dependency_list_export, status: status) } + context 'when export type is dependency_list' do + let(:export_type) { :dependency_list } + + context 'when the exportable is a project' do + let_it_be(:project) { create(:project) } + let(:expected_filename) do + [ + project.full_path.parameterize, + '_dependencies_', + Time.current.utc.strftime('%FT%H%M'), + '.', + 'json' + ].join + end + + it_behaves_like 'export service', Dependencies::ExportSerializers::ProjectDependenciesService do + let(:dependency_list_export) do + create(:dependency_list_export, exportable: project, status: status, export_type: export_type) + end + end + end + + context 'when the exportable is a group' do + let_it_be(:group) { create(:group) } + let(:expected_filename) do + [ + group.full_path.parameterize, + '_dependencies_', + Time.current.utc.strftime('%FT%H%M'), + '.', + 'json' + ].join + end + + it_behaves_like 'export service', Dependencies::ExportSerializers::GroupDependenciesService do + let(:dependency_list_export) do + create(:dependency_list_export, exportable: group, status: status, export_type: export_type) + end + end end end - context 'when the exportable is a group' do - let_it_be(:group) { create(:group) } + context 'when export type is sbom' do + let(:export_type) { :sbom } + + context 'when the exportable is a pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let(:expected_filename) do + [ + 'gl-', + 'pipeline-', + pipeline.id, + '-merged-', + Time.current.utc.strftime('%FT%H%M'), + '-sbom.', + 'cdx', + '.', + 'json' + ].join + end - it_behaves_like 'export service', Dependencies::ExportSerializers::GroupDependenciesService do - let(:dependency_list_export) { create(:dependency_list_export, exportable: group, status: status) } + it_behaves_like 'export service', Dependencies::ExportSerializers::Sbom::PipelineSbomService do + let(:dependency_list_export) do + create(:dependency_list_export, exportable: pipeline, status: status, export_type: export_type) + end + end end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index edfb1880dc3a99..4dcca4bdbf9631 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -183,6 +183,30 @@ def authorized_project_scope?(project) current_authenticated_job.project == project end + # rubocop: disable CodeReuse/ActiveRecord + def find_pipeline(id) + return unless id + + if id.to_s =~ INTEGER_ID_REGEX + ::Ci::Pipeline.find_by(id: id) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + def find_pipeline!(id) + pipeline = find_pipeline(id) + check_pipeline_access(pipeline) + end + + def check_pipeline_access(pipeline) + return forbidden! unless authorized_project_scope?(pipeline&.project) + + return pipeline if can?(current_user, :read_pipeline, pipeline) + return unauthorized! if authenticate_non_public? + + not_found!('Pipeline') + end + # rubocop: disable CodeReuse/ActiveRecord def find_group(id) if id.to_s =~ INTEGER_ID_REGEX diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index ec63c4af13c885..b41ab1dff3fcb9 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -247,6 +247,165 @@ def app end end + describe '#find_pipeline' do + let(:pipeline) { create(:ci_pipeline) } + + shared_examples 'pipeline finder' do + context 'when pipeline exists' do + it 'returns requested pipeline' do + expect(helper.find_pipeline(existing_id)).to eq(pipeline) + end + end + + context 'when pipeline does not exists' do + it 'returns nil' do + expect(helper.find_pipeline(non_existing_id)).to be_nil + end + end + + context 'when pipeline id is not provided' do + it 'returns nil' do + expect(helper.find_pipeline(nil)).to be_nil + end + end + end + + context 'when ID is used as an argument' do + let(:existing_id) { pipeline.id } + let(:non_existing_id) { non_existing_record_id } + + it_behaves_like 'pipeline finder' + end + + context 'when string ID is used as an argument' do + let(:existing_id) { pipeline.id.to_s } + let(:non_existing_id) { non_existing_record_id } + + it_behaves_like 'pipeline finder' + end + + context 'when ID is a negative number' do + let(:existing_id) { pipeline.id } + let(:non_existing_id) { -1 } + + it_behaves_like 'pipeline finder' + end + end + + describe '#find_pipeline!' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:user) { create(:user) } + + shared_examples 'private project without access' do + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value('private')) + allow(helper).to receive(:authenticate_non_public?).and_return(false) + end + + it 'returns not found' do + expect(helper).to receive(:not_found!) + + helper.find_pipeline!(pipeline.id) + end + end + + context 'when user is authenticated' do + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:initial_current_user).and_return(user) + end + + context 'public project' do + it 'returns requested pipeline' do + expect(helper.find_pipeline!(pipeline.id)).to eq(pipeline) + end + end + + context 'private project' do + it_behaves_like 'private project without access' + + context 'without read pipeline permission' do + before do + allow(helper).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(false) + end + + it_behaves_like 'private project without access' + end + end + + context 'with read pipeline permission' do + before do + allow(helper).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(true) + end + + it 'returns requested pipeline' do + expect(helper.find_pipeline!(pipeline.id)).to eq(pipeline) + end + end + end + + context 'when user is not authenticated' do + before do + allow(helper).to receive(:current_user).and_return(nil) + allow(helper).to receive(:initial_current_user).and_return(nil) + end + + context 'public project' do + it 'returns requested pipeline' do + expect(helper.find_pipeline!(pipeline.id)).to eq(pipeline) + end + end + + context 'private project' do + it_behaves_like 'private project without access' + end + end + + context 'support for IDs and paths as argument' do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let(:user) { project.first_owner } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:authorized_project_scope?).and_return(true) + allow(helper).to receive(:job_token_authentication?).and_return(false) + allow(helper).to receive(:authenticate_non_public?).and_return(false) + end + + shared_examples 'pipeline finder' do + context 'when pipeline exists' do + it 'returns requested pipeline' do + expect(helper.find_pipeline!(existing_id)).to eq(pipeline) + end + + it 'returns nil' do + expect(helper).to receive(:render_api_error!).with('404 Pipeline Not Found', 404) + expect(helper.find_pipeline!(non_existing_id)).to be_nil + end + end + end + + context 'when ID is used as an argument' do + context 'when pipeline id is an integer' do + let(:existing_id) { pipeline.id } + let(:non_existing_id) { non_existing_record_id } + + it_behaves_like 'pipeline finder' + end + + context 'when pipeline id is a string' do + let(:existing_id) { pipeline.id.to_s } + let(:non_existing_id) { "non_existing_record_id" } + + it_behaves_like 'pipeline finder' + end + end + end + end + describe '#find_group!' do let_it_be(:group) { create(:group, :public) } let_it_be(:user) { create(:user) } diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index 8a5b80e3051717..54f6297c6ff97e 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -142,5 +142,30 @@ end end end + + describe 'read_dependencies' do + let(:project) { create(:project, :repository) } + + before do + project.add_developer(user) + allow(policy).to receive(:can?).with(:read_dependencies, project).and_return(can_read_project_dependencies) + end + + context 'when user is allowed to read project dependencies' do + let(:can_read_project_dependencies) { true } + + it 'is enabled' do + expect(policy).to be_allowed :read_dependencies + end + end + + context 'when user is not allowed to read project dependencies' do + let(:can_read_project_dependencies) { false } + + it 'is disabled' do + expect(policy).not_to be_allowed :read_dependencies + end + end + end end end -- GitLab From e252274f5ea62ade856cb53e893bb05193b7c366 Mon Sep 17 00:00:00 2001 From: atiwari71 Date: Mon, 4 Sep 2023 16:33:28 +0530 Subject: [PATCH 2/3] Ack review comments --- app/policies/ci/pipeline_policy.rb | 8 ++++---- .../{pipeline_sbom_service.rb => pipeline_service.rb} | 6 +++++- ee/app/services/dependencies/export_service.rb | 4 ++-- ...line_sbom_service_spec.rb => pipeline_service_spec.rb} | 8 +++----- ee/spec/services/dependencies/export_service_spec.rb | 5 ++++- spec/policies/ci/pipeline_policy_spec.rb | 8 ++++---- 6 files changed, 22 insertions(+), 17 deletions(-) rename ee/app/services/dependencies/export_serializers/sbom/{pipeline_sbom_service.rb => pipeline_service.rb} (82%) rename ee/spec/services/dependencies/export_serializers/sbom/{pipeline_sbom_service_spec.rb => pipeline_service_spec.rb} (86%) diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index d3fd059658d7a7..1d60b1e79def8b 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -18,8 +18,8 @@ class PipelinePolicy < BasePolicy @subject.triggered_by?(@user) end - condition(:project_allows_read_dependencies) do - can?(:read_dependencies, @subject.project) + condition(:project_allows_read_dependency) do + can?(:read_dependency, @subject.project) end # Disallow users without permissions from accessing internal pipelines @@ -45,8 +45,8 @@ class PipelinePolicy < BasePolicy enable :read_pipeline_variable end - rule { project_allows_read_dependencies }.policy do - enable :read_dependencies + rule { project_allows_read_dependency }.policy do + enable :read_dependency end def ref_protected?(user, project, tag, ref) diff --git a/ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb b/ee/app/services/dependencies/export_serializers/sbom/pipeline_service.rb similarity index 82% rename from ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb rename to ee/app/services/dependencies/export_serializers/sbom/pipeline_service.rb index 5004b6ecb3d135..7edfa951942df5 100644 --- a/ee/app/services/dependencies/export_serializers/sbom/pipeline_sbom_service.rb +++ b/ee/app/services/dependencies/export_serializers/sbom/pipeline_service.rb @@ -3,9 +3,13 @@ module Dependencies module ExportSerializers module Sbom - class PipelineSbomService + class PipelineService SchemaValidationError = Class.new(StandardError) + # Creates and execute the service. + # + # @param dependency_list_export [Dependencies::DependencyListExport] + # @return [Sbom::SbomEntity] with sbom information of a pipeline. def self.execute(dependency_list_export) new(dependency_list_export).execute end diff --git a/ee/app/services/dependencies/export_service.rb b/ee/app/services/dependencies/export_service.rb index 793227c5306166..dfd340f5a40f0f 100644 --- a/ee/app/services/dependencies/export_service.rb +++ b/ee/app/services/dependencies/export_service.rb @@ -8,7 +8,7 @@ class ExportService Group => ExportSerializers::GroupDependenciesService }, sbom: { - Ci::Pipeline => ExportSerializers::Sbom::PipelineSbomService + Ci::Pipeline => ExportSerializers::Sbom::PipelineService } }.with_indifferent_access.freeze @@ -39,7 +39,7 @@ def create_export create_export_file dependency_list_export.finish! - rescue StandardError, Dependencies::ExportSerializers::Sbom::PipelineSbomService::SchemaValidationError + rescue StandardError, Dependencies::ExportSerializers::Sbom::PipelineService::SchemaValidationError dependency_list_export.reset_state! raise diff --git a/ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb b/ee/spec/services/dependencies/export_serializers/sbom/pipeline_service_spec.rb similarity index 86% rename from ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb rename to ee/spec/services/dependencies/export_serializers/sbom/pipeline_service_spec.rb index 31f575fdba8eb1..994d5f9e962e93 100644 --- a/ee/spec/services/dependencies/export_serializers/sbom/pipeline_sbom_service_spec.rb +++ b/ee/spec/services/dependencies/export_serializers/sbom/pipeline_service_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe Dependencies::ExportSerializers::Sbom::PipelineSbomService, feature_category: :dependency_management do - let(:pipeline) { create(:ee_ci_pipeline, :with_cyclonedx_report) } +RSpec.describe Dependencies::ExportSerializers::Sbom::PipelineService, feature_category: :dependency_management do + let_it_be(:pipeline) { create(:ee_ci_pipeline, :with_cyclonedx_report) } describe '.execute' do let(:dependency_list_export) { instance_double(Dependencies::DependencyListExport, pipeline: pipeline) } @@ -24,15 +24,13 @@ let(:service_class) { described_class.new(dependency_list_export) } - let(:components) { service_class.execute.as_json[:components] } + subject(:components) { service_class.execute.as_json[:components] } before do stub_licensed_features(dependency_scanning: true) end context 'when the pipeline does not have cyclonedx reports' do - subject { components } - let_it_be(:pipeline) { create(:ee_ci_pipeline) } it { is_expected.to be_empty } diff --git a/ee/spec/services/dependencies/export_service_spec.rb b/ee/spec/services/dependencies/export_service_spec.rb index d7f7e57cfa87f5..9fd853d985bd09 100644 --- a/ee/spec/services/dependencies/export_service_spec.rb +++ b/ee/spec/services/dependencies/export_service_spec.rb @@ -87,6 +87,7 @@ context 'when the exportable is a project' do let_it_be(:project) { create(:project) } + let(:expected_filename) do [ project.full_path.parameterize, @@ -106,6 +107,7 @@ context 'when the exportable is a group' do let_it_be(:group) { create(:group) } + let(:expected_filename) do [ group.full_path.parameterize, @@ -129,6 +131,7 @@ context 'when the exportable is a pipeline' do let_it_be(:pipeline) { create(:ci_pipeline) } + let(:expected_filename) do [ 'gl-', @@ -143,7 +146,7 @@ ].join end - it_behaves_like 'export service', Dependencies::ExportSerializers::Sbom::PipelineSbomService do + it_behaves_like 'export service', Dependencies::ExportSerializers::Sbom::PipelineService do let(:dependency_list_export) do create(:dependency_list_export, exportable: pipeline, status: status, export_type: export_type) end diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index 54f6297c6ff97e..e74bf8f7efad97 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -143,19 +143,19 @@ end end - describe 'read_dependencies' do + describe 'read_dependency' do let(:project) { create(:project, :repository) } before do project.add_developer(user) - allow(policy).to receive(:can?).with(:read_dependencies, project).and_return(can_read_project_dependencies) + allow(policy).to receive(:can?).with(:read_dependency, project).and_return(can_read_project_dependencies) end context 'when user is allowed to read project dependencies' do let(:can_read_project_dependencies) { true } it 'is enabled' do - expect(policy).to be_allowed :read_dependencies + expect(policy).to be_allowed :read_dependency end end @@ -163,7 +163,7 @@ let(:can_read_project_dependencies) { false } it 'is disabled' do - expect(policy).not_to be_allowed :read_dependencies + expect(policy).not_to be_allowed :read_dependency end end end -- GitLab From 50c05ad67d8ef2cf8c3e0570bbdf635516aa861b Mon Sep 17 00:00:00 2001 From: atiwari71 Date: Wed, 6 Sep 2023 15:21:07 +0530 Subject: [PATCH 3/3] Fixes feature flag --- ee/lib/api/dependency_list_exports.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/lib/api/dependency_list_exports.rb b/ee/lib/api/dependency_list_exports.rb index 6317b916c347d4..58188629e6b254 100644 --- a/ee/lib/api/dependency_list_exports.rb +++ b/ee/lib/api/dependency_list_exports.rb @@ -47,7 +47,7 @@ class DependencyListExports < ::API::Base end desc 'Generate a dependency list export on a pipeline-level' post ':id/dependency_list_exports' do - not_found! unless Feature.enabled?(:merge_sbom_api, user_pipeline) + not_found! unless Feature.enabled?(:merge_sbom_api, user_pipeline&.project) # Currently, we only support sbom export type for this endpoint. not_found! if params[:export_type] != 'sbom' -- GitLab