Commit 8d40f35a authored by Igor Frenkel's avatar Igor Frenkel Committed by Heinrich Lee Yu

Use allowlist when importing projects

parent 97153f9b
...@@ -15,6 +15,7 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do ...@@ -15,6 +15,7 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
:deploy_access_levels | true :deploy_access_levels | true
:protected_environments | true :protected_environments | true
:security_setting | true :security_setting | true
:project | true
end end
with_them do with_them do
...@@ -27,7 +28,11 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do ...@@ -27,7 +28,11 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
subject { described_class.new } subject { described_class.new }
additional_attributes = { user: %w[id] } # these are attributes for which either a special exception is made or are available only via included modules and not attribute introspection
additional_attributes = {
user: %w[id],
project: %w[auto_devops_deploy_strategy auto_devops_enabled issues_enabled jobs_enabled merge_requests_enabled snippets_enabled wiki_enabled build_git_strategy build_enabled security_and_compliance_enabled requirements_enabled]
}
Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes| Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes|
context "for #{relation_sym}" do context "for #{relation_sym}" do
......
...@@ -106,12 +106,7 @@ module Gitlab ...@@ -106,12 +106,7 @@ module Gitlab
def update_params! def update_params!
params = @importable_attributes.except(*relations.keys.map(&:to_s)) params = @importable_attributes.except(*relations.keys.map(&:to_s))
params = params.merge(present_override_params) params = params.merge(present_override_params)
params = filter_attributes(params)
# Cleaning all imported and overridden params
params = Gitlab::ImportExport::AttributeCleaner.clean(
relation_hash: params,
relation_class: importable_class,
excluded_keys: excluded_keys_for_relation(importable_class_sym))
@importable.assign_attributes(params) @importable.assign_attributes(params)
...@@ -122,6 +117,25 @@ module Gitlab ...@@ -122,6 +117,25 @@ module Gitlab
end end
end end
def filter_attributes(params)
if use_attributes_permitter? && attributes_permitter.permitted_attributes_defined?(importable_class_sym)
attributes_permitter.permit(importable_class_sym, params)
else
Gitlab::ImportExport::AttributeCleaner.clean(
relation_hash: params,
relation_class: importable_class,
excluded_keys: excluded_keys_for_relation(importable_class_sym))
end
end
def attributes_permitter
@attributes_permitter ||= Gitlab::ImportExport::AttributesPermitter.new
end
def use_attributes_permitter?
Feature.enabled?(:permitted_attributes_for_import_export, default_enabled: :yaml)
end
def present_override_params def present_override_params
# we filter out the empty strings from the overrides # we filter out the empty strings from the overrides
# keeping the default values configured # keeping the default values configured
......
...@@ -630,6 +630,74 @@ included_attributes: ...@@ -630,6 +630,74 @@ included_attributes:
- :expires_at - :expires_at
- :ldap - :ldap
- :override - :override
project:
- :approvals_before_merge
- :archived
- :auto_cancel_pending_pipelines
- :autoclose_referenced_issues
- :build_allow_git_fetch
- :build_coverage_regex
- :build_timeout
- :ci_config_path
- :delete_error
- :description
- :disable_overriding_approvers_per_merge_request
- :external_authorization_classification_label
- :has_external_issue_tracker
- :has_external_wiki
- :issues_template
- :jobs_cache_index
- :last_repository_check_failed
- :merge_requests_author_approval
- :merge_requests_disable_committers_approval
- :merge_requests_ff_only_enabled
- :merge_requests_rebase_enabled
- :merge_requests_template
- :only_allow_merge_if_all_discussions_are_resolved
- :only_allow_merge_if_pipeline_succeeds
- :pages_https_only
- :pending_delete
- :printing_merge_request_link_enabled
- :public_builds
- :remove_source_branch_after_merge
- :request_access_enabled
- :require_password_to_approve
- :reset_approvals_on_push
- :resolve_outdated_diff_discussions
- :service_desk_enabled
- :shared_runners_enabled
- :suggestion_commit_message
- :visibility_level
- :hooks
- :issues_access_level
- :forking_access_level
- :merge_requests_access_level
- :wiki_access_level
- :snippets_access_level
- :builds_access_level
- :repository_access_level
- :pages_access_level
- :metrics_dashboard_access_level
- :analytics_access_level
- :operations_access_level
- :security_and_compliance_access_level
- :container_registry_access_level
- :allow_merge_on_skipped_pipeline
- :auto_devops_deploy_strategy
- :auto_devops_enabled
- :container_registry_enabled
- :issues_enabled
- :jobs_enabled
- :merge_method
- :merge_requests_enabled
- :snippets_enabled
- :squash_option
- :topics
- :visibility
- :wiki_enabled
- :build_git_strategy
- :build_enabled
- :security_and_compliance_enabled
# Do not include the following attributes for the models specified. # Do not include the following attributes for the models specified.
excluded_attributes: excluded_attributes:
...@@ -1017,3 +1085,6 @@ ee: ...@@ -1017,3 +1085,6 @@ ee:
- :auto_fix_dast - :auto_fix_dast
- :auto_fix_dependency_scanning - :auto_fix_dependency_scanning
- :auto_fix_sast - :auto_fix_sast
project:
- :requirements_enabled
- :requirements_access_level
...@@ -140,6 +140,7 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do ...@@ -140,6 +140,7 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
:zoom_meetings | true :zoom_meetings | true
:issues | true :issues | true
:group_members | true :group_members | true
:project | true
end end
with_them do with_them do
...@@ -150,7 +151,11 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do ...@@ -150,7 +151,11 @@ RSpec.describe Gitlab::ImportExport::AttributesPermitter do
describe 'included_attributes for Project' do describe 'included_attributes for Project' do
subject { described_class.new } subject { described_class.new }
additional_attributes = { user: %w[id] } # these are attributes for which either a special exception is made or are available only via included modules and not attribute introspection
additional_attributes = {
user: %w[id],
project: %w[auto_devops_deploy_strategy auto_devops_enabled issues_enabled jobs_enabled merge_requests_enabled snippets_enabled wiki_enabled build_git_strategy build_enabled security_and_compliance_enabled requirements_enabled]
}
Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes| Gitlab::ImportExport::Config.new.to_h[:included_attributes].each do |relation_sym, permitted_attributes|
context "for #{relation_sym}" do context "for #{relation_sym}" do
......
...@@ -26,7 +26,7 @@ RSpec.shared_examples 'a permitted attribute' do |relation_sym, permitted_attrib ...@@ -26,7 +26,7 @@ RSpec.shared_examples 'a permitted attribute' do |relation_sym, permitted_attrib
end end
it 'does not contain attributes that would be cleaned with AttributeCleaner' do it 'does not contain attributes that would be cleaned with AttributeCleaner' do
expect((cleaned_hash.keys + additional_attributes.to_a.map(&:to_s))).to include(*permitted_hash.keys) expect(cleaned_hash.keys + additional_attributes.to_a.map(&:to_s)).to include(*permitted_hash.keys)
end end
it 'does not contain prohibited attributes that are not related to given relation' do it 'does not contain prohibited attributes that are not related to given relation' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment