diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb index db9765230fe2081d2b213c1d0bc07acd9356f619..de161685cf740f50a740c278ae84d97a2e363318 100644 --- a/ee/app/models/sbom/occurrence.rb +++ b/ee/app/models/sbom/occurrence.rb @@ -133,7 +133,8 @@ class Occurrence < ApplicationRecord scope :visible_to, ->(user) do return self if user.can_read_all_resources? - where(project_id: user.project_authorizations.select(:project_id)) + joins(project: [:project_authorizations]) + .where(project_authorizations: { user_id: user.id }) end def location diff --git a/ee/app/services/dependencies/export_serializers/organization_dependencies_service.rb b/ee/app/services/dependencies/export_serializers/organization_dependencies_service.rb index 4194623778c00cf152445dc49e3a83c04e15cef2..9f18df7f8d9f5fc97b6610662d991b68bbe2d31b 100644 --- a/ee/app/services/dependencies/export_serializers/organization_dependencies_service.rb +++ b/ee/app/services/dependencies/export_serializers/organization_dependencies_service.rb @@ -14,7 +14,7 @@ def filename def each yield header - each_batch do |batch| + iterator.each_batch do |batch| build_list_for(batch).each do |occurrence| yield to_csv([ occurrence.component_name, @@ -34,10 +34,23 @@ def header to_csv(%w[Name Version Packager Location]) end - def each_batch - Gitlab::Pagination::Keyset::Iterator - .new(scope: export.organization.sbom_occurrences) - .each_batch { |batch| yield batch } + def iterator + if export.organization.owner?(export.author) || export.author.can_read_all_resources? + Gitlab::Pagination::Keyset::Iterator + .new(scope: export.organization.sbom_occurrences) + else + clazz = ::Sbom::Occurrence + # rubocop: disable CodeReuse/ActiveRecord -- where clause + Gitlab::Pagination::Keyset::Iterator.new( + scope: export.organization.sbom_occurrences.order(:id), + in_operator_optimization_options: { + array_scope: export.author.project_authorizations.select(:project_id), + array_mapping_scope: ->(id) { clazz.where(clazz.arel_table[:project_id].eq(id)) }, + finder_query: ->(id) { clazz.where(clazz.arel_table[:id].eq(id)) } + } + ) + # rubocop: enable CodeReuse/ActiveRecord + end end def build_list_for(batch) diff --git a/ee/spec/requests/explore/dependencies_controller_spec.rb b/ee/spec/requests/explore/dependencies_controller_spec.rb index 0e4db5d55d2c01dd1b22c2084adb7d60f109d341..7a802818404939ababad790d7314773ddbf186c2 100644 --- a/ee/spec/requests/explore/dependencies_controller_spec.rb +++ b/ee/spec/requests/explore/dependencies_controller_spec.rb @@ -204,7 +204,7 @@ end end - context 'when user is not admin' do + context 'when the user is not a member of the default organization' do let_it_be(:user) { create(:user, :without_default_org) } before do diff --git a/ee/spec/services/dependencies/export_serializers/organization_dependencies_service_spec.rb b/ee/spec/services/dependencies/export_serializers/organization_dependencies_service_spec.rb index a1d93ec7d70d7b6d2077680f055b05ffcfbba466..c67b37712667afc2e283fe67803a1a052bf95c69 100644 --- a/ee/spec/services/dependencies/export_serializers/organization_dependencies_service_spec.rb +++ b/ee/spec/services/dependencies/export_serializers/organization_dependencies_service_spec.rb @@ -4,8 +4,9 @@ RSpec.describe Dependencies::ExportSerializers::OrganizationDependenciesService, feature_category: :dependency_management do let_it_be(:organization) { create(:organization) } + let_it_be(:user) { create(:user) } let_it_be_with_reload(:project) { create(:project, organization: organization) } - let_it_be(:export) { create(:dependency_list_export, project: nil, organization: organization) } + let_it_be(:export) { create(:dependency_list_export, project: nil, organization: organization, author: user) } let(:service_class) { described_class.new(export) } @@ -26,16 +27,61 @@ create(:sbom_occurrence, :mit, project: project, component: bundler, component_version: bundler_v1) end - it 'includes each occurrence' do - expect(dependencies).to match_array([ - header, - CSV.generate_line([ - occurrence_1.component_name, - occurrence_1.version, - occurrence_1.package_manager, - occurrence_1.location[:blob_path] - ], force_quotes: true) - ]) + context 'when the user is an organization owner' do + let_it_be(:organization_user) { create(:organization_user, :owner, organization: organization, user: user) } + + it 'includes each occurrence', :aggregate_failures do + expect(dependencies.count).to eq(2) + expect(dependencies).to match_array([ + header, + CSV.generate_line([ + occurrence_1.component_name, + occurrence_1.version, + occurrence_1.package_manager, + occurrence_1.location[:blob_path] + ], force_quotes: true) + ]) + end + end + + context 'when the user is an admin', :enable_admin_mode do + before_all do + user.update!(admin: true) + end + + it 'includes each occurrence' do + expect(dependencies).to match_array([ + header, + CSV.generate_line([ + occurrence_1.component_name, + occurrence_1.version, + occurrence_1.package_manager, + occurrence_1.location[:blob_path] + ], force_quotes: true) + ]) + end + end + + context 'when the user has limited access' do + let_it_be(:other_project) { create(:project, organization: organization) } + let_it_be(:visible_occurrence) { create(:sbom_occurrence, project: other_project) } + + before_all do + other_project.add_developer(export.author) + end + + it 'includes each occurrence they have access to', :aggregate_failures do + expect(dependencies.count).to eq(2) + expect(dependencies).to match_array([ + header, + CSV.generate_line([ + visible_occurrence.component_name, + visible_occurrence.version, + visible_occurrence.package_manager, + visible_occurrence.location[:blob_path] + ], force_quotes: true) + ]) + end end it 'avoids N+1 queries' do diff --git a/ee/spec/services/dependencies/export_service_spec.rb b/ee/spec/services/dependencies/export_service_spec.rb index 279aa2019dd1472a2b24ae698b17936ea6c6fc25..450edfb118c83922c178e9f302e04286e9cd6533 100644 --- a/ee/spec/services/dependencies/export_service_spec.rb +++ b/ee/spec/services/dependencies/export_service_spec.rb @@ -92,9 +92,13 @@ let_it_be(:organization) { create(:organization) } let_it_be(:project) { create(:project, organization: organization) } let_it_be(:occurrences) { create_list(:sbom_occurrence, 2, project: project) } - let(:export) { create(:dependency_list_export, project: nil, exportable: organization) } + let_it_be_with_reload(:export) { create(:dependency_list_export, project: nil, exportable: organization) } let(:expected_filename) { "#{organization.to_param}_dependencies_#{timestamp}.csv" } + before_all do + project.add_developer(export.author) + end + it { expect(execute).to be_present } it { expect { execute }.to change { export.file.filename }.to(expected_filename) }