Merge branch 'fix/import-projectmember-security' into 'security'

Fix Import/Export foreign key issue to do with project members

Cleans-up any foreign keys in `ProjectMember` - same as we do with the rest of the models when importing.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/23837 and https://gitlab.com/gitlab-org/gitlab-ce/issues/23739

See merge request !2020

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 29 insertions, 12 deletions

diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb
index f755a404693..34169319b26 100644
--- a/lib/gitlab/import_export/attribute_cleaner.rb
+++ b/lib/gitlab/import_export/attribute_cleaner.rb
@@ -3,10 +3,25 @@ module Gitlab
     class AttributeCleaner
       ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id']
 
-      def self.clean!(relation_hash:)
-        relation_hash.reject! do |key, _value|
-          key.end_with?('_id') && !ALLOWED_REFERENCES.include?(key)
-        end
+      def self.clean(*args)
+        new(*args).clean
+      end
+
+      def initialize(relation_hash:, relation_class:)
+        @relation_hash = relation_hash
+        @relation_class = relation_class
+      end
+
+      def clean
+        @relation_hash.reject do |key, _value|
+          prohibited_key?(key) || !@relation_class.attribute_method?(key)
+        end.except('id')
+      end
+
+      private
+
+      def prohibited_key?(key)
+        key.end_with?('_id') && !ALLOWED_REFERENCES.include?(key)
       end
     end
   end
diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb
index 36c4cf6efa0..b790733f4a7 100644
--- a/lib/gitlab/import_export/members_mapper.rb
+++ b/lib/gitlab/import_export/members_mapper.rb
@@ -55,7 +55,12 @@ module Gitlab
       end
 
       def member_hash(member)
-        member.except('id').merge(source_id: @project.id, importing: true)
+        parsed_hash(member).merge('source_id' => @project.id, 'importing' => true)
+      end
+
+      def parsed_hash(member)
+        Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: member.deep_stringify_keys,
+                                                     relation_class: ProjectMember)
       end
 
       def find_project_user_query(member)
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index dc630e76411..a0e80fccad9 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -14,7 +14,7 @@ module Gitlab
                     priorities: :label_priorities,
                     label: :project_label }.freeze
 
-      USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id].freeze
+      USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id created_by_id].freeze
 
       PROJECT_REFERENCES = %w[project_id source_project_id gl_project_id target_project_id].freeze
 
@@ -30,7 +30,7 @@ module Gitlab
 
       def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project_id:)
         @relation_name = OVERRIDES[relation_sym] || relation_sym
-        @relation_hash = relation_hash.except('id', 'noteable_id').merge('project_id' => project_id)
+        @relation_hash = relation_hash.except('noteable_id').merge('project_id' => project_id)
         @members_mapper = members_mapper
         @user = user
         @imported_object_retries = 0
@@ -172,11 +172,8 @@ module Gitlab
       end
 
       def parsed_relation_hash
-        @parsed_relation_hash ||= begin
-          Gitlab::ImportExport::AttributeCleaner.clean!(relation_hash: @relation_hash)
-
-          @relation_hash.reject { |k, _v| !relation_class.attribute_method?(k) }
-        end
+        @parsed_relation_hash ||= Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: @relation_hash,
+                                                                               relation_class: relation_class)
       end
 
       def set_st_diffs