From cfb511ea692fedffbad5b6ee1a7638b1eb78a4d6 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Thu, 27 Oct 2016 10:22:18 +0000
Subject: [PATCH] Merge branch 'fix/import-projectmember-security' into
 'security'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 lib/gitlab/import_export/attribute_cleaner.rb | 23 +++++++++++++++----
 lib/gitlab/import_export/members_mapper.rb    |  7 +++++-
 lib/gitlab/import_export/relation_factory.rb  | 11 ++++-----
 .../import_export/attribute_cleaner_spec.rb   |  7 ++++--
 4 files changed, 34 insertions(+), 14 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
diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
index b8e7932eb4a..63bab0f0d0d 100644
--- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
+++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
@@ -1,8 +1,10 @@
 require 'spec_helper'
 
 describe Gitlab::ImportExport::AttributeCleaner, lib: true do
+  let(:relation_class){ double('relation_class').as_null_object }
   let(:unsafe_hash) do
     {
+      'id' => 101,
       'service_id' => 99,
       'moved_to_id' => 99,
       'namespace_id' => 99,
@@ -27,8 +29,9 @@ describe Gitlab::ImportExport::AttributeCleaner, lib: true do
   end
 
   it 'removes unwanted attributes from the hash' do
-    described_class.clean!(relation_hash: unsafe_hash)
+    # allow(relation_class).to receive(:attribute_method?).and_return(true)
+    parsed_hash = described_class.clean(relation_hash: unsafe_hash, relation_class: relation_class)
 
-    expect(unsafe_hash).to eq(post_safe_hash)
+    expect(parsed_hash).to eq(post_safe_hash)
   end
 end