From e589c7e84854bd181d5059200e80f0b5c0b0e5f8 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Jan 2017 12:06:09 +0100 Subject: Ignore encrypted attributes in Import/Export * Regenerates tokens for all models that have them * Remove variables, since they are basically just storing encrypted data * Bumped version up to 0.1.6 * Updated related docs --- app/views/projects/edit.html.haml | 2 ++ changelogs/unreleased/fix-import-encrypt-atts.yml | 4 ++++ doc/user/project/settings/import_export.md | 6 +++++- lib/gitlab/import_export.rb | 2 +- lib/gitlab/import_export/import_export.yml | 1 - lib/gitlab/import_export/relation_factory.rb | 15 ++++++++++----- spec/lib/gitlab/import_export/project.json | 13 +++++++++---- .../gitlab/import_export/project_tree_restorer_spec.rb | 14 ++++++++++++++ 8 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 changelogs/unreleased/fix-import-encrypt-atts.yml diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 84787155168..ec944d4ffb7 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -183,6 +183,8 @@ %li Build traces and artifacts %li LFS objects %li Container registry images + %li CI variables + %li Any encrypted tokens %hr - if can? current_user, :archive_project, @project .row.prepend-top-default diff --git a/changelogs/unreleased/fix-import-encrypt-atts.yml b/changelogs/unreleased/fix-import-encrypt-atts.yml new file mode 100644 index 00000000000..e34d895570b --- /dev/null +++ b/changelogs/unreleased/fix-import-encrypt-atts.yml @@ -0,0 +1,4 @@ +--- +title: Ignore encrypted attributes in Import/Export +merge_request: +author: diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index dfc762fe1d3..cb1c1a84f8c 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -22,7 +22,8 @@ with all their related data and be moved into a new GitLab instance. | GitLab version | Import/Export version | | -------- | -------- | -| 8.13.0 to current | 0.1.5 | +| 8.16.2 to current | 0.1.6 | +| 8.13.0 | 0.1.5 | | 8.12.0 | 0.1.4 | | 8.10.3 | 0.1.3 | | 8.10.0 | 0.1.2 | @@ -47,6 +48,9 @@ The following items will NOT be exported: - Build traces and artifacts - LFS objects +- Container registry images +- CI variables +- Any encrypted tokens ## Exporting a project and its data diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index eb667a85b78..d679edec36b 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -3,7 +3,7 @@ module Gitlab extend self # For every version update, the version history in import_export.md has to be kept up to date. - VERSION = '0.1.5' + VERSION = '0.1.6' FILENAME_LIMIT = 50 def export_path(relative_path:) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 08ad3274b38..416194e57d7 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -39,7 +39,6 @@ project_tree: - :author - :events - :statuses - - :variables - :triggers - :deploy_keys - :services diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 19e43cce768..040a10011e2 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -4,7 +4,6 @@ module Gitlab OVERRIDES = { snippets: :project_snippets, pipelines: 'Ci::Pipeline', statuses: 'commit_status', - variables: 'Ci::Variable', triggers: 'Ci::Trigger', builds: 'Ci::Build', hooks: 'ProjectHook', @@ -24,6 +23,8 @@ module Gitlab EXISTING_OBJECT_CHECK = %i[milestone milestones label labels project_label project_labels group_label group_labels].freeze + TOKEN_RESET_MODELS = %w[Ci::Trigger Ci::Build ProjectHook] + def self.create(*args) new(*args).create end @@ -61,7 +62,8 @@ module Gitlab update_project_references handle_group_label if group_label? - reset_ci_tokens if @relation_name == 'Ci::Trigger' + reset_ci_tokens! + @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data'] set_st_diffs if @relation_name == :merge_request_diff end @@ -140,11 +142,14 @@ module Gitlab end end - def reset_ci_tokens - return unless Gitlab::ImportExport.reset_tokens? + def reset_ci_tokens! + return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name.to_s) # If we import/export a project to the same instance, tokens will have to be reset. - @relation_hash['token'] = nil + # We also have to reset them to avoid issues when the gitlab secrets file cannot be copied across. + relation_class.attribute_names.select { |name| name.include?('token') }.each do |token| + @relation_hash[token] = nil + end end def relation_class diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 2c0750c3377..2e9f60432b4 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -6980,12 +6980,17 @@ } ] } - ], - "variables": [ - ], "triggers": [ - + { + "id": 123, + "token": "cdbfasdf44a5958c83654733449e585", + "project_id": null, + "deleted_at": null, + "created_at": "2017-01-16T15:25:28.637Z", + "updated_at": "2017-01-16T15:25:28.637Z", + "gl_project_id": 123 + } ], "deploy_keys": [ diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 4b07fa53bf5..75c8b997b4f 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -197,6 +197,20 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do expect(restored_project_json).to be true end end + + context 'tokens are regenerated' do + before do + restored_project_json + end + + it 'has a new CI triggers token' do + expect(Ci::Trigger.where(token: 'cdbfasdf44a5958c83654733449e585')).to be_empty + end + + it 'has a new CI triggers token' do + expect(Ci::Build.where(token: 'abcd')).to be_empty + end + end end end end -- cgit v1.2.1 From 6a2b976c66b17ec62575bfefda99221ccdc85e1e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Jan 2017 12:11:12 +0100 Subject: fix typo --- lib/gitlab/import_export/relation_factory.rb | 2 +- spec/lib/gitlab/import_export/project_tree_restorer_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 040a10011e2..8a44e78fb44 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -23,7 +23,7 @@ module Gitlab EXISTING_OBJECT_CHECK = %i[milestone milestones label labels project_label project_labels group_label group_labels].freeze - TOKEN_RESET_MODELS = %w[Ci::Trigger Ci::Build ProjectHook] + TOKEN_RESET_MODELS = %w[Ci::Trigger Ci::Build ProjectHook].freeze def self.create(*args) new(*args).create diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 75c8b997b4f..40d7d59f03b 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -203,11 +203,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do restored_project_json end - it 'has a new CI triggers token' do + it 'has a new CI trigger token' do expect(Ci::Trigger.where(token: 'cdbfasdf44a5958c83654733449e585')).to be_empty end - it 'has a new CI triggers token' do + it 'has a new CI build token' do expect(Ci::Build.where(token: 'abcd')).to be_empty end end -- cgit v1.2.1 From bb2201267550433f3db6a49086076ca3937da191 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 24 Jan 2017 14:15:49 +0100 Subject: fix spec failures --- .../import_export/test_project_export.tar.gz | Bin 682154 -> 681799 bytes .../gitlab/import_export/relation_factory_spec.rb | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz index 7655c2b351f..20cdfbae24f 100644 Binary files a/spec/features/projects/import_export/test_project_export.tar.gz and b/spec/features/projects/import_export/test_project_export.tar.gz differ diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index db0084d6823..d381316c7fe 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -55,8 +55,8 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do expect(created_object.project_id).to eq(project.id) end - it 'has a token' do - expect(created_object.token).to eq(token) + it 'has a nil token' do + expect(created_object.token).to eq(nil) end context 'original service exists' do -- cgit v1.2.1 From eeb13c16d1f627eba10313bf5c4662416392feb3 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 25 Jan 2017 10:17:50 +0100 Subject: rename method and added note to export file spec about new encrypted attributes --- lib/gitlab/import_export/relation_factory.rb | 4 ++-- spec/features/projects/import_export/export_file_spec.rb | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 8a44e78fb44..cb9f20a70a0 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -62,7 +62,7 @@ module Gitlab update_project_references handle_group_label if group_label? - reset_ci_tokens! + reset_tokens! @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data'] set_st_diffs if @relation_name == :merge_request_diff @@ -142,7 +142,7 @@ module Gitlab end end - def reset_ci_tokens! + def reset_tokens! return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name.to_s) # If we import/export a project to the same instance, tokens will have to be reset. diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb index 52d08982c7a..2fbfe045b7f 100644 --- a/spec/features/projects/import_export/export_file_spec.rb +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -74,6 +74,9 @@ feature 'Import/Export - project export integration test', feature: true, js: tr Otherwise, please add the exception to +safe_list+ in CURRENT_SPEC using #{sensitive_word} as the key and the correspondent hash or model as the value. + Also, if the attribute is encrypted, please add it to either the list of excluded attributes in IMPORT_EXPORT_CONFIG + or the model to RelationFactory::TOKEN_RESET_MODELS if it includes a token that can be reset. + IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} CURRENT_SPEC: #{__FILE__} MSG -- cgit v1.2.1 From 1a2d13c821179a2707790d9c0e4585044462cd42 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 30 Jan 2017 12:26:49 +0100 Subject: programmatically remove encrypted attributes. Added relevant spec. --- lib/gitlab/import_export/relation_factory.rb | 9 +++++++++ spec/features/projects/import_export/export_file_spec.rb | 4 ++-- spec/lib/gitlab/import_export/relation_factory_spec.rb | 11 +++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index cb9f20a70a0..dd7f9e2fd65 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -63,6 +63,7 @@ module Gitlab handle_group_label if group_label? reset_tokens! + remove_encrypted_attributes! @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data'] set_st_diffs if @relation_name == :merge_request_diff @@ -152,6 +153,14 @@ module Gitlab end end + def remove_encrypted_attributes! + return if relation_class.encrypted_attributes.empty? + + relation_class.encrypted_attributes.each_key do |key| + @relation_hash[key.to_s] = nil + end + end + def relation_class @relation_class ||= @relation_name.to_s.classify.constantize end diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb index 2fbfe045b7f..16dddb2a86b 100644 --- a/spec/features/projects/import_export/export_file_spec.rb +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -74,8 +74,8 @@ feature 'Import/Export - project export integration test', feature: true, js: tr Otherwise, please add the exception to +safe_list+ in CURRENT_SPEC using #{sensitive_word} as the key and the correspondent hash or model as the value. - Also, if the attribute is encrypted, please add it to either the list of excluded attributes in IMPORT_EXPORT_CONFIG - or the model to RelationFactory::TOKEN_RESET_MODELS if it includes a token that can be reset. + Also, if the attribute is a generated unique token, please add it to RelationFactory::TOKEN_RESET_MODELS if it needs to be + reset (to prevent duplicate column problems while importing to the same instance). IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} CURRENT_SPEC: #{__FILE__} diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index d381316c7fe..c6370f3c164 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -178,4 +178,15 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do expect(created_object.author).to eq(new_user) end end + + context 'encrypted attributes' do + let(:relation_sym) { 'Ci::Variable' } + let(:relation_hash) do + create(:ci_variable).as_json + end + + it 'maps the right author to the imported note' do + expect(created_object.value).to be_nil + end + end end -- cgit v1.2.1 From bd6b13c620f1f6447715240fe6c6b865ef7336f0 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 30 Jan 2017 12:33:06 +0100 Subject: fix typo in spec --- spec/lib/gitlab/import_export/relation_factory_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index c6370f3c164..57e412b0cef 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -185,7 +185,7 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do create(:ci_variable).as_json end - it 'maps the right author to the imported note' do + it 'has no value for the encrypted attribute' do expect(created_object.value).to be_nil end end -- cgit v1.2.1 From 017a5068f60c0a7fb058422c5d8d2740f36084a4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 30 Jan 2017 14:26:05 +0100 Subject: fix spec failure --- lib/gitlab/import_export/relation_factory.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index dd7f9e2fd65..0319d7707a8 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -154,7 +154,7 @@ module Gitlab end def remove_encrypted_attributes! - return if relation_class.encrypted_attributes.empty? + return unless relation_class.respond_to?(:encrypted_attributes) && relation_class.encrypted_attributes.any? relation_class.encrypted_attributes.each_key do |key| @relation_hash[key.to_s] = nil -- cgit v1.2.1