From 14a6539348a75a3f83785e37be57ad46ff81eaae Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 30 Aug 2022 07:50:56 +0200 Subject: [PATCH 1/4] Filter out inaccessible issue epics When exporting a project, it's possible that some epics referenced by issues won't be accessible by user (who is exporting the project). We need to filter out such epics. --- app/models/issue.rb | 4 ++ .../bulk_imports/relation_export_service.rb | 2 +- .../bulk_imports/tree_export_service.rb | 8 ++- doc/development/import_export.md | 20 +++++++ ee/app/models/ee/issue.rb | 7 +++ .../import_export/project/tree_saver_spec.rb | 56 +++++++++---------- ee/spec/models/issue_spec.rb | 43 ++++++++++++++ lib/gitlab/import_export/attributes_finder.rb | 4 +- lib/gitlab/import_export/group/tree_saver.rb | 3 +- .../json/streaming_serializer.rb | 24 ++++++-- .../import_export/project/import_export.yml | 9 +++ .../import_export/project/relation_saver.rb | 3 +- .../import_export/project/tree_saver.rb | 3 +- .../json/streaming_serializer_spec.rb | 49 +++++++++++++++- spec/models/issue_spec.rb | 8 +++ .../bulk_imports/tree_export_service_spec.rb | 4 +- 16 files changed, 202 insertions(+), 45 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 153747c75df915..d59b9fad4b2315 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -641,6 +641,10 @@ def expire_etag_cache Gitlab::EtagCaching::Store.new.touch(key) end + def exportable_association?(key, current_user) + false + end + private def due_date_after_start_date diff --git a/app/services/bulk_imports/relation_export_service.rb b/app/services/bulk_imports/relation_export_service.rb index c43f0d8cb4fa38..b1efa8811800b3 100644 --- a/app/services/bulk_imports/relation_export_service.rb +++ b/app/services/bulk_imports/relation_export_service.rb @@ -65,7 +65,7 @@ def remove_existing_export_file!(export) def export_service @export_service ||= if config.tree_relation?(relation) || config.self_relation?(relation) - TreeExportService.new(portable, config.export_path, relation) + TreeExportService.new(portable, config.export_path, relation, user) elsif config.file_relation?(relation) FileExportService.new(portable, config.export_path, relation) else diff --git a/app/services/bulk_imports/tree_export_service.rb b/app/services/bulk_imports/tree_export_service.rb index 8e885e590d1b7e..b6f094da5585df 100644 --- a/app/services/bulk_imports/tree_export_service.rb +++ b/app/services/bulk_imports/tree_export_service.rb @@ -2,11 +2,12 @@ module BulkImports class TreeExportService - def initialize(portable, export_path, relation) + def initialize(portable, export_path, relation, user) @portable = portable @export_path = export_path @relation = relation @config = FileTransfer.config_for(portable) + @user = user end def execute @@ -27,7 +28,7 @@ def exported_filename private - attr_reader :export_path, :portable, :relation, :config + attr_reader :export_path, :portable, :relation, :config, :user # rubocop: disable CodeReuse/Serializer def serializer @@ -35,7 +36,8 @@ def serializer portable, config.portable_tree, json_writer, - exportable_path: '' + exportable_path: '', + current_user: user ) end # rubocop: enable CodeReuse/Serializer diff --git a/doc/development/import_export.md b/doc/development/import_export.md index 6cbbb6bf716732..f843797a5e556c 100644 --- a/doc/development/import_export.md +++ b/doc/development/import_export.md @@ -305,6 +305,26 @@ export_reorders: nulls_position: :nulls_last ``` +### Conditional Export + +When associated resources are from outside of project, it may be necessary to +validate that user who is exporting the project/group can access these +associations. `include_if_exportable` accepts an array of associations for a +resource. Then during export `exportable_association?` method on the resource +is called with association name and user to validate if associated resource can +be included in export. + +This definition will call issue's `exportable_association?(:epic_issue, +current_user)` method and include issue's epic_issue association for each issue +only if the method returns true: + +```yaml +include_if_exportable: + project: + issues: + - epic_issue +``` + ### Import The import job status moves from `none` to `finished` or `failed` into different states: diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb index 31b1c871e2a060..34bb9d83da9e89 100644 --- a/ee/app/models/ee/issue.rb +++ b/ee/app/models/ee/issue.rb @@ -323,6 +323,13 @@ def has_epic? epic_issue.present? end + override :exportable_association? + def exportable_association?(key, current_user) + return false unless key == :epic_issue + + epic.present? && current_user&.can?(:read_epic, epic) + end + private def blocking_issues_ids diff --git a/ee/spec/lib/ee/gitlab/import_export/project/tree_saver_spec.rb b/ee/spec/lib/ee/gitlab/import_export/project/tree_saver_spec.rb index d354e9e2fc2121..72ff98ab790a2f 100644 --- a/ee/spec/lib/ee/gitlab/import_export/project/tree_saver_spec.rb +++ b/ee/spec/lib/ee/gitlab/import_export/project/tree_saver_spec.rb @@ -7,7 +7,6 @@ let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:shared) { project.import_export_shared } let_it_be(:note2) { create(:note, noteable: issue, project: project, author: user) } let_it_be(:epic) { create(:epic, group: group) } @@ -17,16 +16,10 @@ let_it_be(:push_rule) { create(:push_rule, project: project, max_file_size: 10) } - after :all do - FileUtils.rm_rf(export_path) - end - shared_examples 'EE saves project tree successfully' do |ndjson_enabled| include ::ImportExport::CommonUtil - let_it_be(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } - - let_it_be(:full_path) do + let(:full_path) do if ndjson_enabled File.join(shared.export_path, 'tree') else @@ -34,36 +27,41 @@ end end - let_it_be(:exportable_path) { 'project' } - - before_all do - RSpec::Mocks.with_temporary_scope do - stub_all_feature_flags - stub_feature_flags(project_export_as_ndjson: ndjson_enabled) - project.add_maintainer(user) - - expect(project_tree_saver.save).to be true - end + let(:shared) { project.import_export_shared } + let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } + let(:issue_json) { get_json(full_path, exportable_path, :issues, ndjson_enabled).first } + let(:exportable_path) { 'project' } + let(:epics_available) { true } + + before do + stub_all_feature_flags + stub_feature_flags(project_export_as_ndjson: ndjson_enabled) + stub_licensed_features(epics: epics_available) + project.add_maintainer(user) end - let_it_be(:issue_json) { get_json(full_path, exportable_path, :issues, ndjson_enabled).first } + after do + FileUtils.rm_rf(export_path) + FileUtils.rm_rf(full_path) + end context 'epics' do - it 'has epic_issue' do + it 'contains issue epic object', :aggregate_failures do + expect(project_tree_saver.save).to be true expect(issue_json['epic_issue']).not_to be_empty expect(issue_json['epic_issue']['id']).to eql(epic_issue.id) - end - - it 'has epic' do expect(issue_json['epic_issue']['epic']['title']).to eql(epic.title) - end - - it 'does not have epic_id' do expect(issue_json['epic_issue']['epic_id']).to be_nil + expect(issue_json['epic_issue']['issue_id']).to be_nil end - it 'does not have issue_id' do - expect(issue_json['epic_issue']['issue_id']).to be_nil + context 'when epic is not readable' do + let(:epics_available) { false } + + it 'filters out inaccessible epic object' do + expect(project_tree_saver.save).to be true + expect(issue_json['epic_issue']).to be_nil + end end end @@ -74,6 +72,7 @@ end it 'has security settings' do + expect(project_tree_saver.save).to be true expect(security_json['auto_fix_dependency_scanning']).to be_truthy end end @@ -85,6 +84,7 @@ end it 'has push rules' do + expect(project_tree_saver.save).to be true expect(push_rule_json['max_file_size']).to eq(10) expect(push_rule_json['force_push_regex']).to eq('feature\/.*') end diff --git a/ee/spec/models/issue_spec.rb b/ee/spec/models/issue_spec.rb index 45f48e23c04622..cbecc943c6f0b2 100644 --- a/ee/spec/models/issue_spec.rb +++ b/ee/spec/models/issue_spec.rb @@ -1297,4 +1297,47 @@ it { is_expected.to eq true } end end + + describe '#exportable_association?' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :private) } + let_it_be(:project) { create(:project, group: group) } + let_it_be_with_reload(:issue) { create(:issue, project: project) } + let_it_be(:epic) { create(:epic, group: group) } + let_it_be(:epic_issue) { create(:epic_issue, issue: issue, epic: epic) } + + let(:key) { :epic_issue } + + subject { issue.exportable_association?(key, user) } + + it { is_expected.to be_falsey } + + context 'when epics are available' do + before do + stub_licensed_features(epics: true) + end + + it { is_expected.to be_falsey } + + context 'when user can read epic' do + before do + group.add_developer(user) + end + + it { is_expected.to be_truthy } + + context 'for an unknown key' do + let(:key) { :labels } + + it { is_expected.to be_falsey } + end + + context 'for an unauthenticated user' do + let(:user) { nil } + + it { is_expected.to be_falsey } + end + end + end + end end diff --git a/lib/gitlab/import_export/attributes_finder.rb b/lib/gitlab/import_export/attributes_finder.rb index 4abc3da11909b1..8843b4f5755655 100644 --- a/lib/gitlab/import_export/attributes_finder.rb +++ b/lib/gitlab/import_export/attributes_finder.rb @@ -12,6 +12,7 @@ def initialize(config:) @methods = config[:methods] || {} @preloads = config[:preloads] || {} @export_reorders = config[:export_reorders] || {} + @include_if_exportable = config[:include_if_exportable] || {} end def find_root(model_key) @@ -35,7 +36,8 @@ def find(model_key, model_tree) methods: @methods[model_key], include: resolve_model_tree(model_tree), preload: resolve_preloads(model_key, model_tree), - export_reorder: @export_reorders[model_key] + export_reorder: @export_reorders[model_key], + include_if_exportable: @include_if_exportable[model_key] }.compact end diff --git a/lib/gitlab/import_export/group/tree_saver.rb b/lib/gitlab/import_export/group/tree_saver.rb index 796b9258e57b0d..b4c86c3fc7f046 100644 --- a/lib/gitlab/import_export/group/tree_saver.rb +++ b/lib/gitlab/import_export/group/tree_saver.rb @@ -46,7 +46,8 @@ def serialize(group) group, group_tree, json_writer, - exportable_path: "groups/#{group.id}" + exportable_path: "groups/#{group.id}", + current_user: @current_user ).execute end diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index 42e014e91f8877..4bf17b9f7350fb 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -14,8 +14,9 @@ def to_json(*_args) end end - def initialize(exportable, relations_schema, json_writer, exportable_path:, logger: Gitlab::Export::Logger) + def initialize(exportable, relations_schema, json_writer, current_user:, exportable_path:, logger: Gitlab::Export::Logger) @exportable = exportable + @current_user = current_user @exportable_path = exportable_path @relations_schema = relations_schema @json_writer = json_writer @@ -59,7 +60,7 @@ def serialize_relation(definition) private - attr_reader :json_writer, :relations_schema, :exportable, :logger + attr_reader :json_writer, :relations_schema, :exportable, :logger, :current_user def serialize_many_relations(key, records, options) log_relation_export(key, records.size) @@ -73,7 +74,7 @@ def serialize_many_relations(key, records, options) batch.each do |record| before_read_callback(record) - items << Raw.new(record.to_json(options)) + items << exportable_json_record(record, options, key) after_read_callback(record) end @@ -83,6 +84,19 @@ def serialize_many_relations(key, records, options) json_writer.write_relation_array(@exportable_path, key, enumerator) end + def exportable_json_record(record, options, key) + export_check = relations_schema[:include_if_exportable]&.dig(key) + return Raw.new(record.to_json(options)) unless export_check && options[:include] + + filtered_options = options.deep_dup + export_check.each do |check_key| + association = check_key.to_sym + filtered_options[:include].delete_if { |option| option.has_key?(association) && !record.exportable_association?(association, current_user) } + end + + Raw.new(record.to_json(filtered_options)) + end + def batch(relation, key) opts = { of: BATCH_SIZE } order_by = reorders(relation, key) @@ -111,7 +125,7 @@ def serialize_many_each(key, records, options) enumerator = Enumerator.new do |items| records.each do |record| - items << Raw.new(record.to_json(options)) + items << exportable_json_record(record, options, key) end end @@ -121,7 +135,7 @@ def serialize_many_each(key, records, options) def serialize_single_relation(key, record, options) log_relation_export(key) - json = Raw.new(record.to_json(options)) + json = exportable_json_record(record, options, key) json_writer.write_relation(@exportable_path, key, json) end diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 77311e14803368..310f95fe921c0c 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -1155,3 +1155,12 @@ ee: - :user_id - :action - :created_at + + preloads: + issues: + epic: + + include_if_exportable: + project: + issues: + - epic_issue diff --git a/lib/gitlab/import_export/project/relation_saver.rb b/lib/gitlab/import_export/project/relation_saver.rb index b40827e36f8dba..8e91adac196659 100644 --- a/lib/gitlab/import_export/project/relation_saver.rb +++ b/lib/gitlab/import_export/project/relation_saver.rb @@ -32,7 +32,8 @@ def serializer project, reader.project_tree, json_writer, - exportable_path: 'project' + exportable_path: 'project', + current_user: nil ) end diff --git a/lib/gitlab/import_export/project/tree_saver.rb b/lib/gitlab/import_export/project/tree_saver.rb index 1b54e4b975e4d4..bd34cd3ff6e13b 100644 --- a/lib/gitlab/import_export/project/tree_saver.rb +++ b/lib/gitlab/import_export/project/tree_saver.rb @@ -50,7 +50,8 @@ def stream_export reader.project_tree, json_writer, exportable_path: "project", - logger: @logger + logger: @logger, + current_user: @current_user ) Retriable.retriable(on: Net::OpenTimeout, on_retry: on_retry) do diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index b8d18718dfbd07..18560e0b51d789 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -32,18 +32,20 @@ let(:hash) { { name: exportable.name, description: exportable.description }.stringify_keys } let(:include) { [] } let(:custom_orderer) { nil } + let(:include_if_exportable) { {} } let(:relations_schema) do { only: [:name, :description], include: include, preload: { issues: nil }, - export_reorder: custom_orderer + export_reorder: custom_orderer, + include_if_exportable: include_if_exportable } end subject do - described_class.new(exportable, relations_schema, json_writer, exportable_path: exportable_path, logger: logger) + described_class.new(exportable, relations_schema, json_writer, exportable_path: exportable_path, logger: logger, current_user: user) end describe '#execute' do @@ -210,6 +212,49 @@ subject.execute end end + + describe 'conditional export of included associations' do + let(:include) do + [{ issues: { include: [{ label_links: { include: [:label] } }] } }] + end + + let(:include_if_exportable) do + { issues: [:label_links] } + end + + let_it_be(:label) { create(:label, project: exportable) } + let_it_be(:link) { create(:label_link, label: label, target: issue) } + + context 'when association is exportable' do + before do + allow_next_found_instance_of(Issue) do |issue| + allow(issue).to receive(:exportable_association?).with(:label_links, user).and_return(true) + end + end + + it 'includes exportable association' do + expected_issue = issue.to_json(include: [{ label_links: { include: [:label] } }]) + + expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, array_including(expected_issue)) + + subject.execute + end + end + + context 'when association is not exportable' do + before do + allow_next_found_instance_of(Issue) do |issue| + allow(issue).to receive(:exportable_association?).with(:label_links, user).and_return(false) + end + end + + it 'filters out not exportable association' do + expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, array_including(issue.to_json)) + + subject.execute + end + end + end end describe '#serialize_relation' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 17c3cd17364c70..d1f12ba0aaf37e 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1771,6 +1771,14 @@ end end + describe '#exportable_association?' do + it 'returns false' do + issue = build(:issue) + + expect(issue.exportable_association?(:labels, user)).to be_falsey + end + end + context 'order by closed_at' do let!(:issue_a) { create(:issue, closed_at: 1.day.ago) } let!(:issue_b) { create(:issue, closed_at: 5.days.ago) } diff --git a/spec/services/bulk_imports/tree_export_service_spec.rb b/spec/services/bulk_imports/tree_export_service_spec.rb index ffb81fe2b5f6ef..6e26cb6dc2b3d3 100644 --- a/spec/services/bulk_imports/tree_export_service_spec.rb +++ b/spec/services/bulk_imports/tree_export_service_spec.rb @@ -8,7 +8,7 @@ let(:relation) { 'issues' } - subject(:service) { described_class.new(project, export_path, relation) } + subject(:service) { described_class.new(project, export_path, relation, project.owner) } describe '#execute' do it 'executes export service and archives exported data' do @@ -21,7 +21,7 @@ context 'when unsupported relation is passed' do it 'raises an error' do - service = described_class.new(project, export_path, 'unsupported') + service = described_class.new(project, export_path, 'unsupported', project.owner) expect { service.execute }.to raise_error(BulkImports::Error, 'Unsupported relation export type') end -- GitLab From 4c1d2a76f2d8dc5f252d9018f608f892c51a9df2 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Thu, 8 Sep 2022 08:38:33 +0200 Subject: [PATCH 2/4] Check if record responds to exportable method If method is not available, do not export the resource. --- app/models/issue.rb | 4 ---- ee/app/models/ee/issue.rb | 1 - .../import_export/json/streaming_serializer.rb | 18 +++++++++++++----- .../json/streaming_serializer_spec.rb | 14 ++++++++++++++ spec/models/issue_spec.rb | 8 -------- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index d59b9fad4b2315..153747c75df915 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -641,10 +641,6 @@ def expire_etag_cache Gitlab::EtagCaching::Store.new.touch(key) end - def exportable_association?(key, current_user) - false - end - private def due_date_after_start_date diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb index 34bb9d83da9e89..9ce293fe958310 100644 --- a/ee/app/models/ee/issue.rb +++ b/ee/app/models/ee/issue.rb @@ -323,7 +323,6 @@ def has_epic? epic_issue.present? end - override :exportable_association? def exportable_association?(key, current_user) return false unless key == :epic_issue diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index 4bf17b9f7350fb..5517d70ef66495 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -85,18 +85,26 @@ def serialize_many_relations(key, records, options) end def exportable_json_record(record, options, key) - export_check = relations_schema[:include_if_exportable]&.dig(key) - return Raw.new(record.to_json(options)) unless export_check && options[:include] + associations = relations_schema[:include_if_exportable]&.dig(key) + return Raw.new(record.to_json(options)) unless associations && options[:include] filtered_options = options.deep_dup - export_check.each do |check_key| - association = check_key.to_sym - filtered_options[:include].delete_if { |option| option.has_key?(association) && !record.exportable_association?(association, current_user) } + associations.each do |association| + filtered_options[:include].delete_if do |option| + !exportable_json_association?(option, record, association.to_sym) + end end Raw.new(record.to_json(filtered_options)) end + def exportable_json_association?(option, record, association) + return true unless option.has_key?(association) + return false unless record.respond_to?(:exportable_association?) + + record.exportable_association?(association, current_user) + end + def batch(relation, key) opts = { of: BATCH_SIZE } order_by = reorders(relation, key) diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index 18560e0b51d789..c68f7052bc4d48 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -254,6 +254,20 @@ subject.execute end end + + context 'when association does not respond to exportable_association?' do + before do + allow_next_found_instance_of(Issue) do |issue| + allow(issue).to receive(:respond_to?).with(:exportable_association?).and_return(false) + end + end + + it 'filters out not exportable association' do + expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, array_including(issue.to_json)) + + subject.execute + end + end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index d1f12ba0aaf37e..17c3cd17364c70 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1771,14 +1771,6 @@ end end - describe '#exportable_association?' do - it 'returns false' do - issue = build(:issue) - - expect(issue.exportable_association?(:labels, user)).to be_falsey - end - end - context 'order by closed_at' do let!(:issue_a) { create(:issue, closed_at: 1.day.ago) } let!(:issue_b) { create(:issue, closed_at: 5.days.ago) } -- GitLab From 20594e2fb39ba7953ba41605c63e35ac8b8180d1 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 13 Sep 2022 09:55:12 +0200 Subject: [PATCH 3/4] Address review comments * minor cleanups in methods --- doc/development/import_export.md | 21 +++++++++++-------- ee/app/models/ee/issue.rb | 2 +- ee/spec/models/issue_spec.rb | 2 +- .../json/streaming_serializer.rb | 2 +- .../import_export/project/import_export.yml | 18 +++++++++++++++- .../json/streaming_serializer_spec.rb | 4 ++-- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/doc/development/import_export.md b/doc/development/import_export.md index f843797a5e556c..c66ac0418ac632 100644 --- a/doc/development/import_export.md +++ b/doc/development/import_export.md @@ -305,18 +305,16 @@ export_reorders: nulls_position: :nulls_last ``` -### Conditional Export +### Conditional export -When associated resources are from outside of project, it may be necessary to -validate that user who is exporting the project/group can access these +When associated resources are from outside the project, you might need to +validate that a user who is exporting the project or group can access these associations. `include_if_exportable` accepts an array of associations for a -resource. Then during export `exportable_association?` method on the resource -is called with association name and user to validate if associated resource can -be included in export. +resource. During export, the `exportable_association?` method on the resource +is called with the association's name and user to validate if associated +resource can be included in the export. -This definition will call issue's `exportable_association?(:epic_issue, -current_user)` method and include issue's epic_issue association for each issue -only if the method returns true: +For example: ```yaml include_if_exportable: @@ -325,6 +323,11 @@ include_if_exportable: - epic_issue ``` +This definition: + +1. Calls the issue's `exportable_association?(:epic_issue, current_user: current_user)` method. +1. If the method returns true, includes the issue's `epic_issue` association for the issue. + ### Import The import job status moves from `none` to `finished` or `failed` into different states: diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb index 9ce293fe958310..56148ae617798d 100644 --- a/ee/app/models/ee/issue.rb +++ b/ee/app/models/ee/issue.rb @@ -323,7 +323,7 @@ def has_epic? epic_issue.present? end - def exportable_association?(key, current_user) + def exportable_association?(key, current_user: nil) return false unless key == :epic_issue epic.present? && current_user&.can?(:read_epic, epic) diff --git a/ee/spec/models/issue_spec.rb b/ee/spec/models/issue_spec.rb index cbecc943c6f0b2..114570778f940e 100644 --- a/ee/spec/models/issue_spec.rb +++ b/ee/spec/models/issue_spec.rb @@ -1308,7 +1308,7 @@ let(:key) { :epic_issue } - subject { issue.exportable_association?(key, user) } + subject { issue.exportable_association?(key, current_user: user) } it { is_expected.to be_falsey } diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index 5517d70ef66495..99396d64779e5f 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -102,7 +102,7 @@ def exportable_json_association?(option, record, association) return true unless option.has_key?(association) return false unless record.respond_to?(:exportable_association?) - record.exportable_association?(association, current_user) + record.exportable_association?(association, current_user: current_user) end def batch(relation, key) diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 310f95fe921c0c..069eab7b5bf9a1 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -1160,7 +1160,23 @@ ee: issues: epic: + # When associated resources are from outside the project, you might need to + # validate that a user who is exporting the project or group can access these + # associations. `include_if_exportable` accepts an array of associations for a + # resource. During export, the `exportable_association?` method on the + # resource is called with the association's name and user to validate if + # associated resource can be included in the export. + # + # This definition will call issue's `exportable_association?(:epic_issue, + # current_user: current_user)` method and include issue's epic_issue association + # for each issue only if the method returns true: + # + # Example: + # include_if_exportable: + # project: + # issues: + # - epic_issue include_if_exportable: project: issues: - - epic_issue + - :epic_issue diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index c68f7052bc4d48..02ac8065c9fefc 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -228,7 +228,7 @@ context 'when association is exportable' do before do allow_next_found_instance_of(Issue) do |issue| - allow(issue).to receive(:exportable_association?).with(:label_links, user).and_return(true) + allow(issue).to receive(:exportable_association?).with(:label_links, current_user: user).and_return(true) end end @@ -244,7 +244,7 @@ context 'when association is not exportable' do before do allow_next_found_instance_of(Issue) do |issue| - allow(issue).to receive(:exportable_association?).with(:label_links, user).and_return(false) + allow(issue).to receive(:exportable_association?).with(:label_links, current_user: user).and_return(false) end end -- GitLab From f4c8282b0c1e2426a84b0ade258a57d767c92c5b Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Wed, 14 Sep 2022 12:41:42 +0200 Subject: [PATCH 4/4] Fix failing spec --- spec/lib/gitlab/import_export/config_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/import_export/config_spec.rb b/spec/lib/gitlab/import_export/config_spec.rb index fcb48678b881e8..8f848af8bd35fc 100644 --- a/spec/lib/gitlab/import_export/config_spec.rb +++ b/spec/lib/gitlab/import_export/config_spec.rb @@ -21,10 +21,12 @@ end it 'parses default config' do + expected_keys = [:tree, :excluded_attributes, :included_attributes, :methods, :preloads, :export_reorders] + expected_keys << :include_if_exportable if ee + expect { subject }.not_to raise_error expect(subject).to be_a(Hash) - expect(subject.keys).to contain_exactly( - :tree, :excluded_attributes, :included_attributes, :methods, :preloads, :export_reorders) + expect(subject.keys).to match_array(expected_keys) end end end -- GitLab