From cb682d99126e8df346f4fa406dcc2344fd0558ff Mon Sep 17 00:00:00 2001
From: "Alan (Maciej) Paruszewski" <mparuszewski@gitlab.com>
Date: Fri, 5 Nov 2021 05:48:44 +0000
Subject: [PATCH] Use allowlist of allowed attributes for imported models (part
5)
---
.../attributes_permitter_spec.rb | 4 +-
.../import_export/attributes_permitter.rb | 2 +-
.../import_export/project/import_export.yml | 238 +++++++++++++++---
.../attributes_permitter_spec.rb | 31 ++-
.../attributes_permitter_shared_examples.rb | 4 +-
5 files changed, 236 insertions(+), 43 deletions(-)
diff --git a/ee/spec/lib/gitlab/import_export/attributes_permitter_spec.rb b/ee/spec/lib/gitlab/import_export/attributes_permitter_spec.rb
index 2ddbfe381c225..1cb989c051b36 100644
--- a/ee/spec/lib/gitlab/import_export/attributes_permitter_spec.rb
+++ b/ee/spec/lib/gitlab/import_export/attributes_permitter_spec.rb
@@ -27,9 +27,11 @@
subject { described_class.new }
+ additional_attributes = { user: %w[id] }
+
Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes|
context "for #{relation_sym}" do
- it_behaves_like 'a permitted attribute', relation_sym, permitted_attributes
+ it_behaves_like 'a permitted attribute', relation_sym, permitted_attributes, additional_attributes[relation_sym]
end
end
end
diff --git a/lib/gitlab/import_export/attributes_permitter.rb b/lib/gitlab/import_export/attributes_permitter.rb
index 2d8e25a9f70a3..f6f65f855990e 100644
--- a/lib/gitlab/import_export/attributes_permitter.rb
+++ b/lib/gitlab/import_export/attributes_permitter.rb
@@ -44,7 +44,7 @@ class AttributesPermitter
# We want to use AttributesCleaner for these relations instead, in the future this should be removed to make sure
# we are using AttributesPermitter for every imported relation.
- DISABLED_RELATION_NAMES = %i[user author issuable_sla].freeze
+ DISABLED_RELATION_NAMES = %i[author issuable_sla].freeze
def initialize(config: ImportExport::Config.new.to_h)
@config = config
diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml
index 71a8d747963a6..fa1279ca1203a 100644
--- a/lib/gitlab/import_export/project/import_export.yml
+++ b/lib/gitlab/import_export/project/import_export.yml
@@ -178,17 +178,7 @@ included_attributes:
- :project_id
- :key
- :value
- label:
- - :title
- - :color
- - :project_id
- - :group_id
- - :created_at
- - :updated_at
- - :template
- - :description
- - :priority
- labels:
+ label: &label_definition
- :title
- :color
- :project_id
@@ -198,23 +188,13 @@ included_attributes:
- :template
- :description
- :priority
+ labels: *label_definition
priorities:
- :project_id
- :priority
- :created_at
- :updated_at
- milestone:
- - :iid
- - :title
- - :project_id
- - :group_id
- - :description
- - :due_date
- - :created_at
- - :updated_at
- - :start_date
- - :state
- milestones:
+ milestone: &milestone_definition
- :iid
- :title
- :project_id
@@ -225,6 +205,7 @@ included_attributes:
- :updated_at
- :start_date
- :state
+ milestones: *milestone_definition
protected_branches:
- :project_id
- :name
@@ -315,6 +296,200 @@ included_attributes:
- :project_id
- :issue_template_key
- :project_key
+ snippets:
+ - :title
+ - :content
+ - :author_id
+ - :project_id
+ - :created_at
+ - :updated_at
+ - :file_name
+ - :visibility_level
+ - :description
+ project_members:
+ - :access_level
+ - :source_type
+ - :user_id
+ - :notification_level
+ - :created_at
+ - :updated_at
+ - :created_by_id
+ - :invite_email
+ - :invite_accepted_at
+ - :requested_at
+ - :expires_at
+ - :ldap
+ - :override
+ merge_request: &merge_request_definition
+ - :target_branch
+ - :source_branch
+ - :source_project_id
+ - :author_id
+ - :assignee_id
+ - :title
+ - :created_at
+ - :updated_at
+ - :state
+ - :merge_status
+ - :target_project_id
+ - :iid
+ - :description
+ - :updated_by_id
+ - :merge_error
+ - :merge_params
+ - :merge_when_pipeline_succeeds
+ - :merge_user_id
+ - :merge_commit_sha
+ - :squash_commit_sha
+ - :in_progress_merge_commit_sha
+ - :lock_version
+ - :approvals_before_merge
+ - :rebase_commit_sha
+ - :time_estimate
+ - :squash
+ - :last_edited_at
+ - :last_edited_by_id
+ - :discussion_locked
+ - :allow_maintainer_to_push
+ - :merge_ref_sha
+ - :draft
+ - :diff_head_sha
+ - :source_branch_sha
+ - :target_branch_sha
+ merge_requests: *merge_request_definition
+ award_emoji:
+ - :user_id
+ - :name
+ - :awardable_type
+ - :created_at
+ - :updated_at
+ commit_author:
+ - :name
+ - :email
+ committer:
+ - :name
+ - :email
+ events:
+ - :target_type
+ - :action
+ - :author_id
+ - :fingerprint
+ - :created_at
+ - :updated_at
+ label_links:
+ - :target_type
+ - :created_at
+ - :updated_at
+ merge_request_diff:
+ - :state
+ - :created_at
+ - :updated_at
+ - :base_commit_sha
+ - :real_size
+ - :head_commit_sha
+ - :start_commit_sha
+ - :commits_count
+ - :files_count
+ - :sorted
+ - :diff_type
+ merge_request_diff_commits:
+ - :relative_order
+ - :sha
+ - :authored_date
+ - :committed_date
+ - :message
+ - :trailers
+ merge_request_diff_files:
+ - :relative_order
+ - :new_file
+ - :renamed_file
+ - :deleted_file
+ - :new_path
+ - :old_path
+ - :a_mode
+ - :b_mode
+ - :too_large
+ - :binary
+ - :diff
+ metrics:
+ - :created_at
+ - :updated_at
+ - :latest_closed_by_id
+ - :latest_closed_at
+ - :merged_by_id
+ - :merged_at
+ - :latest_build_started_at
+ - :latest_build_finished_at
+ - :first_deployed_to_production_at
+ - :first_comment_at
+ - :first_commit_at
+ - :last_commit_at
+ - :diff_size
+ - :modified_paths_size
+ - :commits_count
+ - :first_approved_at
+ - :first_reassigned_at
+ - :added_lines
+ - :target_project_id
+ - :removed_lines
+ notes:
+ - :note
+ - :noteable_type
+ - :author_id
+ - :created_at
+ - :updated_at
+ - :project_id
+ - :attachment
+ - :line_code
+ - :commit_id
+ - :system
+ - :st_diff
+ - :updated_by_id
+ - :type
+ - :position
+ - :original_position
+ - :change_position
+ - :resolved_at
+ - :resolved_by_id
+ - :resolved_by_push
+ - :discussion_id
+ - :confidential
+ - :last_edited_at
+ push_event_payload:
+ - :commit_count
+ - :action
+ - :ref_type
+ - :commit_from
+ - :commit_to
+ - :ref
+ - :commit_title
+ - :ref_count
+ resource_label_events:
+ - :action
+ - :user_id
+ - :created_at
+ suggestions:
+ - :relative_order
+ - :applied
+ - :commit_id
+ - :from_content
+ - :to_content
+ - :outdated
+ - :lines_above
+ - :lines_below
+ system_note_metadata:
+ - :commit_count
+ - :action
+ - :created_at
+ - :updated_at
+ timelogs:
+ - :time_spent
+ - :user_id
+ - :project_id
+ - :spent_at
+ - :created_at
+ - :updated_at
+ - :summary
# Do not include the following attributes for the models specified.
excluded_attributes:
@@ -434,16 +609,7 @@ excluded_attributes:
- :service_desk_reply_to
- :upvotes_count
- :work_item_type_id
- merge_request:
- - :milestone_id
- - :sprint_id
- - :ref_fetched
- - :merge_jid
- - :rebase_jid
- - :latest_merge_request_diff_id
- - :head_pipeline_id
- - :state_id
- merge_requests:
+ merge_request: &merge_request_excluded_definition
- :milestone_id
- :sprint_id
- :ref_fetched
@@ -452,6 +618,7 @@ excluded_attributes:
- :latest_merge_request_diff_id
- :head_pipeline_id
- :state_id
+ merge_requests: *merge_request_excluded_definition
award_emoji:
- :awardable_id
statuses:
@@ -520,10 +687,9 @@ excluded_attributes:
- :issue_id
zoom_meetings:
- :issue_id
- design:
- - :issue_id
- designs:
+ design: &design_excluded_definition
- :issue_id
+ designs: *design_excluded_definition
design_versions:
- :issue_id
actions:
diff --git a/spec/lib/gitlab/import_export/attributes_permitter_spec.rb b/spec/lib/gitlab/import_export/attributes_permitter_spec.rb
index 55422ec3ef260..4fea61ec3a82f 100644
--- a/spec/lib/gitlab/import_export/attributes_permitter_spec.rb
+++ b/spec/lib/gitlab/import_export/attributes_permitter_spec.rb
@@ -80,8 +80,8 @@
let(:attributes_permitter) { described_class.new }
- where(:relation_name, :permitted_attributes_defined) do
- :user | false
+ where(:relation_name, :permitted_attributes_defined ) do
+ :user | true
:author | false
:ci_cd_settings | true
:metrics_setting | true
@@ -91,6 +91,7 @@
:auto_devops | true
:boards | true
:custom_attributes | true
+ :label | true
:labels | true
:protected_branches | true
:protected_tags | true
@@ -99,6 +100,28 @@
:push_access_levels | true
:releases | true
:links | true
+ :priorities | true
+ :milestone | true
+ :milestones | true
+ :snippets | true
+ :project_members | true
+ :merge_request | true
+ :merge_requests | true
+ :award_emoji | true
+ :commit_author | true
+ :committer | true
+ :events | true
+ :label_links | true
+ :merge_request_diff | true
+ :merge_request_diff_commits | true
+ :merge_request_diff_files | true
+ :metrics | true
+ :notes | true
+ :push_event_payload | true
+ :resource_label_events | true
+ :suggestions | true
+ :system_note_metadata | true
+ :timelogs | true
:container_expiration_policy | true
:project_feature | true
:prometheus_metrics | true
@@ -113,9 +136,11 @@
describe 'included_attributes for Project' do
subject { described_class.new }
+ additional_attributes = { user: %w[id] }
+
Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes|
context "for #{relation_sym}" do
- it_behaves_like 'a permitted attribute', relation_sym, permitted_attributes
+ it_behaves_like 'a permitted attribute', relation_sym, permitted_attributes, additional_attributes[relation_sym]
end
end
end
diff --git a/spec/support/shared_examples/lib/gitlab/import_export/attributes_permitter_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/import_export/attributes_permitter_shared_examples.rb
index 5ce698c470182..41d3d76b66b6b 100644
--- a/spec/support/shared_examples/lib/gitlab/import_export/attributes_permitter_shared_examples.rb
+++ b/spec/support/shared_examples/lib/gitlab/import_export/attributes_permitter_shared_examples.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true
-RSpec.shared_examples 'a permitted attribute' do |relation_sym, permitted_attributes|
+RSpec.shared_examples 'a permitted attribute' do |relation_sym, permitted_attributes, additional_attributes = []|
let(:prohibited_attributes) { %i[remote_url my_attributes my_ids token my_id test] }
let(:import_export_config) { Gitlab::ImportExport::Config.new.to_h }
@@ -26,7 +26,7 @@
end
it 'does not contain attributes that would be cleaned with AttributeCleaner' do
- expect(cleaned_hash.keys).to include(*permitted_hash.keys)
+ expect(cleaned_hash.keys + additional_attributes.to_a).to include(*permitted_hash.keys)
end
it 'does not contain prohibited attributes that are not related to given relation' do
--
GitLab