From be834c4de93a7716035f5373210ea3922c26da72 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 11 Apr 2016 11:13:51 +0200 Subject: [PATCH] changed a few things based on feedback --- app/models/project_import_data.rb | 8 +++++-- ...port_credentials_to_project_import_data.rb | 4 ++-- ...8_remove_wrong_import_url_from_projects.rb | 21 ++++++++----------- spec/factories/project_import_data.rb | 6 ------ 4 files changed, 17 insertions(+), 22 deletions(-) delete mode 100644 spec/factories/project_import_data.rb diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index a0994312003..79efb403058 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -12,7 +12,11 @@ require 'file_size_validator' class ProjectImportData < ActiveRecord::Base belongs_to :project - attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true, mode: :per_attribute_iv_and_salt + attr_encrypted :credentials, + key: Gitlab::Application.secrets.db_key_base, + marshal: true, + encode: true, + mode: :per_attribute_iv_and_salt serialize :data, JSON @@ -21,7 +25,7 @@ class ProjectImportData < ActiveRecord::Base before_validation :symbolize_credentials def symbolize_credentials - # bang doesn't work here + # bang doesn't work here - attr_encrypted makes it not to work self.credentials = self.credentials.deep_symbolize_keys unless self.credentials.blank? end end diff --git a/db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb b/db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb index dd2b3613983..ffcd64266e3 100644 --- a/db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb +++ b/db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb @@ -1,7 +1,7 @@ class AddImportCredentialsToProjectImportData < ActiveRecord::Migration def change add_column :project_import_data, :encrypted_credentials, :text - add_column :project_import_data, :encrypted_credentials_iv, :text - add_column :project_import_data, :encrypted_credentials_salt, :text + add_column :project_import_data, :encrypted_credentials_iv, :string + add_column :project_import_data, :encrypted_credentials_salt, :string end end diff --git a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb index 93e040fce28..dd2842b835e 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -3,7 +3,7 @@ # #down method not supported class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration - class FakeProjectImportData + class ProjectImportDataFake extend AttrEncrypted attr_accessor :credentials attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true, :mode => :per_attribute_iv_and_salt @@ -13,14 +13,11 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration say("Encrypting and migrating project import credentials...") # This should cover GitHub, GitLab, Bitbucket user:password, token@domain, and other similar URLs. - say("Projects and GitHub projects with a wrong URL. It also migrates GitLab project credentials.") - in_transaction { process_projects_with_wrong_url } + in_transaction(message: "Projects including GitHub and GitLab projects with an unsecured URL.") { process_projects_with_wrong_url } - say("Migrating Bitbucket credentials...") - in_transaction { process_project(import_type: 'bitbucket', credentials_keys: ['bb_session']) } + in_transaction(message: "Migrating Bitbucket credentials...") { process_project(import_type: 'bitbucket', credentials_keys: ['bb_session']) } - say("Migrating FogBugz credentials...") - in_transaction { process_project(import_type: 'fogbugz', credentials_keys: ['fb_session']) } + in_transaction(message: "Migrating FogBugz credentials...") { process_project(import_type: 'fogbugz', credentials_keys: ['fb_session']) } end @@ -33,7 +30,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end end - def process_project(import_type: , credentials_keys: []) + def process_project(import_type:, credentials_keys: []) unencrypted_import_data(import_type: import_type).each do |data| replace_data_credentials(data, credentials_keys) end @@ -56,8 +53,8 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration new_data_hash.deep_symbolize_keys end - def in_transaction - say_with_time("Processing new transaction...") do + def in_transaction(message:) + say_with_time(message) do ActiveRecord::Base.transaction do yield end @@ -65,7 +62,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def update_import_data(import_url, project) - fake_import_data = FakeProjectImportData.new + fake_import_data = ProjectImportDataFake.new fake_import_data.credentials = import_url.credentials import_data_id = project['import_data_id'] if import_data_id @@ -76,7 +73,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def update_with_encrypted_data(data_hash, import_data_id, unencrypted_data = ' NULL ') - fake_import_data = FakeProjectImportData.new + fake_import_data = ProjectImportDataFake.new fake_import_data.credentials = data_hash execute(update_import_data_sql(import_data_id, fake_import_data, unencrypted_data)) end diff --git a/spec/factories/project_import_data.rb b/spec/factories/project_import_data.rb deleted file mode 100644 index 9e08d5a22e9..00000000000 --- a/spec/factories/project_import_data.rb +++ /dev/null @@ -1,6 +0,0 @@ -FactoryGirl.define do - factory :project_import_data, class: ProjectImportData do - data "test" - project - end -end