Skip to content

Add AttributesPermitter for Import attributes filtering

George Koltsov requested to merge georgekoltsov/add-attributes-permitter into master

What does this MR do?

This MR adds new AttributesPermitter for Group/Project Import/Export, that is not currently used. It builds a hash of permitted attributes for every model defined in import_export.yml that is used to validate and filter out any attributes that are not permitted when doing Import.

It is not currently used, and a transition from using AttributeCleaner (read - blacklist approach) to AttributesPermitter (whitelist approach) is going to be done gradually, in the follow up MRs. Main idea is to replace AttributeCleaner with AttributesPermitter, in batches, 2-5 models at a time per MR.

pry(main)> Gitlab::ImportExport::AttributesPermitter.new.permitted_attributes

=> {:group_members=>[:user], :user=>[:id, :email, :username], :project=>[:labels, :milestones, :issues, :snippets, :releases, :project_members, :merge_requests, :external_pull_requests, :ci_pipelines, :auto_devops, :triggers, :pipeline_schedules, :container_expiration_policy, :services, :protected_branches, :protected_tags, :project_feature, :custom_attributes, :prometheus_metrics, :project_badges, :ci_cd_settings, :error_tracking_setting, :metrics_setting, :boards, :protected_environments, :service_desk_setting], :service_desk_setting=>[], :protected_environments=>[:deploy_access_levels], :deploy_access_levels=>[], :boards=>[:lists], :lists=>[:label, :list_type], :label=>[:priorities, :priorities, :priorities, :priorities, :priorities, :type], :priorities=>[], :metrics_setting=>[], :error_tracking_setting=>[], :ci_cd_settings=>[:group_runners_enabled], :project_badges=>[:type], :prometheus_metrics=>[], :custom_attributes=>[], :project_feature=>[], :protected_tags=>[:create_access_levels], :create_access_levels=>[], :protected_branches=>[:merge_access_levels, :push_access_levels, :unprotect_access_levels], :unprotect_access_levels=>[], :push_access_levels=>[], :merge_access_levels=>[], :services=>[:type], :container_expiration_policy=>[], :pipeline_schedules=>[], :triggers=>[], :auto_devops=>[], :ci_pipelines=>[:notes, :stages, :external_pull_request, :merge_request, :notes], :merge_request=>[], :external_pull_request=>[], :stages=>[:statuses], :statuses=>[:type], :notes=>[:author, :events, :author, :award_emoji, :system_note_metadata, :events, :suggestions, :author, :award_emoji, :author, :events, :author, :award_emoji, :system_note_metadata, :events, :type], :events=>[:push_event_payload, :push_event_payload, :push_event_payload, :push_event_payload, :push_event_payload, :push_event_payload, :push_event_payload, :push_event_payload, :push_event_payload, :action], :push_event_payload=>[:action], :author=>[:name], :external_pull_requests=>[], :merge_requests=>[:metrics, :award_emoji, :notes, :merge_request_diff, :events, :timelogs, :label_links, :milestone, :resource_label_events, :diff_head_sha, :source_branch_sha, :target_branch_sha, :state], :resource_label_events=>[:label, :label], :milestone=>[:events, :events], :label_links=>[:label, :label], :timelogs=>[], :merge_request_diff=>[:merge_request_diff_commits, :merge_request_diff_files], :merge_request_diff_files=>[:utf8_diff], :merge_request_diff_commits=>[], :suggestions=>[], :system_note_metadata=>[], :award_emoji=>[], :metrics=>[], :project_members=>[:user], :releases=>[:links], :links=>[], :snippets=>[:award_emoji, :notes], :issues=>[:events, :timelogs, :notes, :label_links, :milestone, :resource_label_events, :designs, :design_versions, :issue_assignees, :zoom_meetings, :sentry_issue, :award_emoji, :epic_issue, :state], :epic_issue=>[:epic], :epic=>[], :sentry_issue=>[], :zoom_meetings=>[], :issue_assignees=>[], :design_versions=>[:actions], :actions=>[:design], :design=>[], :designs=>[:notes], :milestones=>[:events], :labels=>[:priorities, :type]}

[106] pry(main)> Gitlab::ImportExport::AttributesPermitter.new.permitted_attributes_for(:project)
=> [:labels, :milestones, :issues, :snippets, :releases, :project_members, :merge_requests, :external_pull_requests, :ci_pipelines, :auto_devops, :triggers, :pipeline_schedules, :container_expiration_policy, :services, :protected_branches, :protected_tags, :project_feature, :custom_attributes, :prometheus_metrics, :project_badges, :ci_cd_settings, :error_tracking_setting, :metrics_setting, :boards, :protected_environments, :service_desk_setting]

[104] pry(main)> Gitlab::ImportExport::AttributesPermitter.new.permit(:project, { id: 123, foo: 'bar', labels: 'test', milestones: 'test'} )
=> {:labels=>"test", :milestones=>"test"}

Right now most of the relations has no permitted attributes. This is due to the fact that included_attributes section in import_export.yml file is empty. In the model transition MRs, for each model, we will have to add an explicit list of whitelisted attributes. And also remove corresponding model's excluded_attributes since it will no longer be needed.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖

Merge request reports