From 18411645505f4bf4bb877743cb4dc027d422414b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 2 Mar 2016 17:31:17 +0100 Subject: add initial migrations --- app/models/project.rb | 1 + ...20160302151724_add_import_credentials_to_projects.rb | 6 ++++++ ...60302152808_remove_wrong_import_url_from_projects.rb | 17 +++++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 db/migrate/20160302151724_add_import_credentials_to_projects.rb create mode 100644 db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb diff --git a/app/models/project.rb b/app/models/project.rb index 6f5d592755a..36b11366f3f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -91,6 +91,7 @@ class Project < ActiveRecord::Base attr_accessor :new_default_branch attr_accessor :old_path_with_namespace + attr_encrypted :import_credentials, key: Gitlab::Application.secrets.db_key_base # Relations belongs_to :creator, foreign_key: 'creator_id', class_name: 'User' diff --git a/db/migrate/20160302151724_add_import_credentials_to_projects.rb b/db/migrate/20160302151724_add_import_credentials_to_projects.rb new file mode 100644 index 00000000000..3cfe2bbd50a --- /dev/null +++ b/db/migrate/20160302151724_add_import_credentials_to_projects.rb @@ -0,0 +1,6 @@ +class AddImportCredentialsToProjects < ActiveRecord::Migration + def change + add_column :projects, :encrypted_import_credentials, :text + add_column :projects, :encrypted_import_credentials_iv, :text + 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 new file mode 100644 index 00000000000..3f1b65aff14 --- /dev/null +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -0,0 +1,17 @@ +class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration + def up + projects_with_wrong_import_url.each do |project| + project.update_columns(import_url: nil) # TODO Check really nil? + # TODO: migrate current credentials to import_credentials? + # TODO: Notify user ? + end + end + + private + + + def projects_with_dot_atom + # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL + select_all("SELECT p.id from projects p WHERE p.import_url LIKE '%//%:%@%' or p.import_url like '#{"_"*40}@github.com%'") + end +end -- cgit v1.2.1 From cefefb2adea23c81c5e6254992da975eca71b559 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 3 Mar 2016 18:10:06 +0100 Subject: WIP - refactored migration and updated project_import_data with encrypted att --- app/models/project.rb | 1 - app/models/project_import_data.rb | 3 +- ...dd_import_credentials_to_project_import_data.rb | 6 ++++ ...302151724_add_import_credentials_to_projects.rb | 6 ---- ...152808_remove_wrong_import_url_from_projects.rb | 36 +++++++++++++++++----- 5 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb delete mode 100644 db/migrate/20160302151724_add_import_credentials_to_projects.rb diff --git a/app/models/project.rb b/app/models/project.rb index 36b11366f3f..6f5d592755a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -91,7 +91,6 @@ class Project < ActiveRecord::Base attr_accessor :new_default_branch attr_accessor :old_path_with_namespace - attr_encrypted :import_credentials, key: Gitlab::Application.secrets.db_key_base # Relations belongs_to :creator, foreign_key: 'creator_id', class_name: 'User' diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index cd3319f077e..2900b86d643 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -12,7 +12,8 @@ require 'file_size_validator' class ProjectImportData < ActiveRecord::Base belongs_to :project - + attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base + serialize :data, JSON validates :project, presence: true 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 new file mode 100644 index 00000000000..ff3d8b466dc --- /dev/null +++ b/db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb @@ -0,0 +1,6 @@ +class AddImportCredentialsToProjectImportData < ActiveRecord::Migration + def change + add_column :project_import_data, :encrypted_credentials, :text + add_column :project_import_data, :encrypted_credentials_iv, :text + end +end diff --git a/db/migrate/20160302151724_add_import_credentials_to_projects.rb b/db/migrate/20160302151724_add_import_credentials_to_projects.rb deleted file mode 100644 index 3cfe2bbd50a..00000000000 --- a/db/migrate/20160302151724_add_import_credentials_to_projects.rb +++ /dev/null @@ -1,6 +0,0 @@ -class AddImportCredentialsToProjects < ActiveRecord::Migration - def change - add_column :projects, :encrypted_import_credentials, :text - add_column :projects, :encrypted_import_credentials_iv, :text - 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 3f1b65aff14..dda7648fb87 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -1,16 +1,38 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration + + class ImportUrlSanitizer + def initialize(url) + @url = url + end + + def sanitized_url + @sanitized_url ||= @url[regex_extractor, 1] + @url[regex_extractor, 3] + end + + def credentials + @credentials ||= @url[regex_extractor, 2] + end + + private + + # Regex matches 1 , 2 , + # 3 + def regex_extractor + /(.*\/\/)(.*)(\@.*)/ + end + end + def up projects_with_wrong_import_url.each do |project| - project.update_columns(import_url: nil) # TODO Check really nil? - # TODO: migrate current credentials to import_credentials? - # TODO: Notify user ? + sanitizer = ImportUrlSanitizer.new(project.import_urls) + project.update_columns(import_url: sanitizer.sanitized_url) + if project.import_data + project.import_data.update_columns(credentials: sanitizer.credentials) + end end end - private - - - def projects_with_dot_atom + def projects_with_wrong_import_url # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL select_all("SELECT p.id from projects p WHERE p.import_url LIKE '%//%:%@%' or p.import_url like '#{"_"*40}@github.com%'") end -- cgit v1.2.1 From 06b36c00d55df38cd2aaa4d5251185485c8abe5c Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 4 Mar 2016 12:21:53 +0100 Subject: some refactoring in the migration. Also fixed github import issue and updated spec --- ...0160302152808_remove_wrong_import_url_from_projects.rb | 15 ++++++++++----- db/schema.rb | 4 +++- lib/gitlab/github_import/project_creator.rb | 12 ++++++++++-- spec/lib/gitlab/github_import/project_creator_spec.rb | 3 ++- 4 files changed, 25 insertions(+), 9 deletions(-) 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 dda7648fb87..dfa9f2d4dee 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -23,11 +23,16 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def up - projects_with_wrong_import_url.each do |project| - sanitizer = ImportUrlSanitizer.new(project.import_urls) - project.update_columns(import_url: sanitizer.sanitized_url) - if project.import_data - project.import_data.update_columns(credentials: sanitizer.credentials) + projects_with_wrong_import_url.each do |project_id| + project = Project.find(project_id["id"]) + sanitizer = ImportUrlSanitizer.new(project.import_url) + + ActiveRecord::Base.transaction do + project.update_columns(import_url: sanitizer.sanitized_url) + if project.import_data + project.import_data.credentials = sanitizer.credentials + project.save! + end end end end diff --git a/db/schema.rb b/db/schema.rb index 53a941d30de..05c19fc7a2e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160222153918) do +ActiveRecord::Schema.define(version: 20160302152808) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -658,6 +658,8 @@ ActiveRecord::Schema.define(version: 20160222153918) do create_table "project_import_data", force: :cascade do |t| t.integer "project_id" t.text "data" + t.text "encrypted_credentials" + t.text "encrypted_credentials_iv" end create_table "projects", force: :cascade do |t| diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index 474927069a5..d6cab3c2d24 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -20,13 +20,21 @@ module Gitlab visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC, import_type: "github", import_source: repo.full_name, - import_url: repo.clone_url.sub("https://", "https://#{@session_data[:github_access_token]}@"), + import_url: repo.clone_url, wiki_enabled: !repo.has_wiki? # If repo has wiki we'll import it later ).execute - project.create_import_data(data: { "github_session" => session_data } ) + create_import_data(project) project end + + private + + def create_import_data(project) + project.create_import_data( + credentials: session_data.delete(:github_access_token), + data: { "github_session" => session_data }) + end end end end diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index c93a3ebdaec..36abe87f527 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -26,7 +26,8 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do project_creator = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, user, access_params) project = project_creator.execute - expect(project.import_url).to eq("https://asdffg@gitlab.com/asd/vim.git") + expect(project.import_url).to eq("https://gitlab.com/asd/vim.git") + expect(project.import_data.credentials).to eq("asdffg") expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end -- cgit v1.2.1 From c2b33d3b71215858892debeb45d93a69d530fd8e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 4 Mar 2016 16:23:19 +0100 Subject: added import url exposer to construct URL withunencrypted credentials --- app/models/project_import_data.rb | 1 + lib/gitlab/github_import/importer.rb | 3 +-- lib/gitlab/github_import/project_creator.rb | 4 ++-- lib/gitlab/github_import/wiki_formatter.rb | 4 +++- lib/gitlab/import_url_exposer.rb | 17 +++++++++++++++++ 5 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 lib/gitlab/import_url_exposer.rb diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 2900b86d643..493c82a94fa 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -13,6 +13,7 @@ require 'file_size_validator' class ProjectImportData < ActiveRecord::Base belongs_to :project attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base + serialize :credentials, JSON serialize :data, JSON diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index e2a85f29825..515fd4720d5 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -7,8 +7,7 @@ module Gitlab def initialize(project) @project = project - import_data = project.import_data.try(:data) - github_session = import_data["github_session"] if import_data + github_session = project.import_data.credentials if import_data @client = Client.new(github_session["github_access_token"]) @formatter = Gitlab::ImportFormatter.new end diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index d6cab3c2d24..b5ed32e5b1e 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -32,8 +32,8 @@ module Gitlab def create_import_data(project) project.create_import_data( - credentials: session_data.delete(:github_access_token), - data: { "github_session" => session_data }) + credentials: { github_access_token: session_data.delete(:github_access_token) }, + data: { github_session: session_data }) end end end diff --git a/lib/gitlab/github_import/wiki_formatter.rb b/lib/gitlab/github_import/wiki_formatter.rb index 6c592ff469c..8be82924107 100644 --- a/lib/gitlab/github_import/wiki_formatter.rb +++ b/lib/gitlab/github_import/wiki_formatter.rb @@ -12,7 +12,9 @@ module Gitlab end def import_url - project.import_url.sub(/\.git\z/, ".wiki.git") + import_url = Gitlab::ImportUrlExposer.expose(import_url: project.import_url, + credentials: project.import_data.credentials) + import_url.sub(/\.git\z/, ".wiki.git") end end end diff --git a/lib/gitlab/import_url_exposer.rb b/lib/gitlab/import_url_exposer.rb new file mode 100644 index 00000000000..6b4af0bf265 --- /dev/null +++ b/lib/gitlab/import_url_exposer.rb @@ -0,0 +1,17 @@ +module Gitlab + # Exposes an import URL that includes the credentials unencrypted. + # Extracted to its own class to prevent unintended use. + module ImportUrlExposer + extend self + + def expose(import_url:, credentials: ) + import_url.sub("//", "//#{parsed_credentials(credentials)}@") + end + + private + + def parsed_credentials(credentials) + credentials.values.join(":") + end + end +end \ No newline at end of file -- cgit v1.2.1 From 7085850c50a6dd7072bd2c80f092b0c20f74d1dc Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 4 Mar 2016 18:37:00 +0100 Subject: fix specs --- app/models/project_import_data.rb | 3 +-- lib/gitlab/import_url_exposer.rb | 2 +- spec/factories/project_import_data.rb | 5 +++++ spec/lib/gitlab/github_import/project_creator_spec.rb | 4 ++-- spec/lib/gitlab/github_import/wiki_formatter_spec.rb | 9 +++++++-- 5 files changed, 16 insertions(+), 7 deletions(-) create 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 493c82a94fa..23f4e97b8aa 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -12,8 +12,7 @@ require 'file_size_validator' class ProjectImportData < ActiveRecord::Base belongs_to :project - attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base - serialize :credentials, JSON + attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true serialize :data, JSON diff --git a/lib/gitlab/import_url_exposer.rb b/lib/gitlab/import_url_exposer.rb index 6b4af0bf265..f1919dffa8a 100644 --- a/lib/gitlab/import_url_exposer.rb +++ b/lib/gitlab/import_url_exposer.rb @@ -14,4 +14,4 @@ module Gitlab credentials.values.join(":") end end -end \ No newline at end of file +end diff --git a/spec/factories/project_import_data.rb b/spec/factories/project_import_data.rb new file mode 100644 index 00000000000..18393cdda98 --- /dev/null +++ b/spec/factories/project_import_data.rb @@ -0,0 +1,5 @@ +FactoryGirl.define do + factory :project_import_data, class: ProjectImportData do + data "test" + end +end \ No newline at end of file diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index 36abe87f527..290c855642a 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do owner: OpenStruct.new(login: "john") ) end - let(:namespace){ create(:group, owner: user) } + let(:namespace) { create(:group, owner: user) } let(:token) { "asdffg" } let(:access_params) { { github_access_token: token } } @@ -27,7 +27,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do project = project_creator.execute expect(project.import_url).to eq("https://gitlab.com/asd/vim.git") - expect(project.import_data.credentials).to eq("asdffg") + expect(project.import_data.credentials).to eq(:github_access_token => "asdffg") expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb index aed2aa39e3a..46d5c2f3296 100644 --- a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -3,10 +3,15 @@ require 'spec_helper' describe Gitlab::GithubImport::WikiFormatter, lib: true do let(:project) do create(:project, namespace: create(:namespace, path: 'gitlabhq'), - import_url: 'https://xxx@github.com/gitlabhq/sample.gitlabhq.git') + import_url: 'https://github.com/gitlabhq/sample.gitlabhq.git') end - subject(:wiki) { described_class.new(project)} + let!(:project_import_data) do + create(:project_import_data, credentials: { github_access_token: 'xxx' }, + project: project) + end + + subject(:wiki) { described_class.new(project) } describe '#path_with_namespace' do it 'appends .wiki to project path' do -- cgit v1.2.1 From 735563329d1f86ee4d72b37cd22eed1168935e8e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 7 Mar 2016 12:50:35 +0100 Subject: refactored a bunch of stuff based on MR feedback --- app/models/project_import_data.rb | 2 +- ...152808_remove_wrong_import_url_from_projects.rb | 38 +++++++++++++--------- lib/gitlab/github_import/importer.rb | 4 +-- lib/gitlab/github_import/project_creator.rb | 3 +- lib/gitlab/import_url_exposer.rb | 14 +++----- spec/lib/gitlab/import_url_exposer_spec.rb | 13 ++++++++ 6 files changed, 44 insertions(+), 30 deletions(-) create mode 100644 spec/lib/gitlab/import_url_exposer_spec.rb diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 23f4e97b8aa..f3b9daa0d1a 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -12,7 +12,7 @@ require 'file_size_validator' class ProjectImportData < ActiveRecord::Base belongs_to :project - attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true + attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true serialize :data, JSON 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 dfa9f2d4dee..881af783c61 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -2,43 +2,49 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration class ImportUrlSanitizer def initialize(url) - @url = url + @url = URI.parse(url) end def sanitized_url - @sanitized_url ||= @url[regex_extractor, 1] + @url[regex_extractor, 3] + @sanitized_url ||= safe_url end def credentials - @credentials ||= @url[regex_extractor, 2] + @credentials ||= { user: @url.user, password: @url.password } end private - # Regex matches 1 , 2 , - # 3 - def regex_extractor - /(.*\/\/)(.*)(\@.*)/ + def safe_url + safe_url = @url.dup + safe_url.password = nil + safe_url.user = nil + safe_url end + + end + + class FakeProjectImportData + extend AttrEncrypted + attr_accessor :credentials + attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true end def up - projects_with_wrong_import_url.each do |project_id| - project = Project.find(project_id["id"]) - sanitizer = ImportUrlSanitizer.new(project.import_url) + projects_with_wrong_import_url.each do |project| + sanitizer = ImportUrlSanitizer.new(project["import_url"]) ActiveRecord::Base.transaction do - project.update_columns(import_url: sanitizer.sanitized_url) - if project.import_data - project.import_data.credentials = sanitizer.credentials - project.save! - end + execute("UPDATE projects SET import_url = '#{sanitizer.sanitized_url}' WHERE id = #{project['id']}") + fake_import_data = FakeProjectImportData.new + fake_import_data.credentials = sanitizer.credentials + execute("UPDATE project_import_data SET encrypted_credentials = '#{fake_import_data.encrypted_credentials}' WHERE project_id = #{project['id']}") end end end def projects_with_wrong_import_url # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL - select_all("SELECT p.id from projects p WHERE p.import_url LIKE '%//%:%@%' or p.import_url like '#{"_"*40}@github.com%'") + select_all("SELECT p.id, p.import_url from projects p WHERE p.import_url LIKE '%//%:%@%' or p.import_url like '#{"_"*40}@github.com%'") end end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 515fd4720d5..d478d3b5398 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -7,8 +7,8 @@ module Gitlab def initialize(project) @project = project - github_session = project.import_data.credentials if import_data - @client = Client.new(github_session["github_access_token"]) + credentials = project.import_data.credentials if import_data + @client = Client.new(credentials["github_access_token"]) @formatter = Gitlab::ImportFormatter.new end diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index b5ed32e5b1e..52aba93a51d 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -32,8 +32,7 @@ module Gitlab def create_import_data(project) project.create_import_data( - credentials: { github_access_token: session_data.delete(:github_access_token) }, - data: { github_session: session_data }) + credentials: { github_access_token: session_data.delete(:github_access_token) }) end end end diff --git a/lib/gitlab/import_url_exposer.rb b/lib/gitlab/import_url_exposer.rb index f1919dffa8a..bf03f5a6daf 100644 --- a/lib/gitlab/import_url_exposer.rb +++ b/lib/gitlab/import_url_exposer.rb @@ -2,16 +2,12 @@ module Gitlab # Exposes an import URL that includes the credentials unencrypted. # Extracted to its own class to prevent unintended use. module ImportUrlExposer - extend self - def expose(import_url:, credentials: ) - import_url.sub("//", "//#{parsed_credentials(credentials)}@") - end - - private - - def parsed_credentials(credentials) - credentials.values.join(":") + def self.expose(import_url:, credentials: ) + uri = URI.parse(import_url) + uri.user = credentials[:user] + uri.password = credentials[:password] + uri end end end diff --git a/spec/lib/gitlab/import_url_exposer_spec.rb b/spec/lib/gitlab/import_url_exposer_spec.rb new file mode 100644 index 00000000000..878947caea1 --- /dev/null +++ b/spec/lib/gitlab/import_url_exposer_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe 'Gitlab::ImportUrlExposer' do + + describe :expose do + let(:credentials) do + Gitlab::ImportUrlExposer.expose(import_url: "https://github.com/me/project.git", credentials: {user: 'blah', password: 'password'}) + end + + it { expect(credentials).to be_a(URI) } + it { expect(credentials.to_s).to eq("https://blah:password@github.com/me/project.git") } + end +end -- cgit v1.2.1 From 383cc8404741f65bd52fbe80eec6c2dae2578fce Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 13:15:51 +0100 Subject: some refactoring based on feedback --- app/models/project.rb | 12 +++++ ...152808_remove_wrong_import_url_from_projects.rb | 52 ++++++++-------------- db/schema.rb | 4 +- lib/gitlab/github_import/importer.rb | 2 +- lib/gitlab/github_import/project_creator.rb | 14 +----- lib/gitlab/github_import/wiki_formatter.rb | 4 +- lib/gitlab/import_url_sanitizer.rb | 24 ++++++++++ 7 files changed, 62 insertions(+), 50 deletions(-) create mode 100644 lib/gitlab/import_url_sanitizer.rb diff --git a/app/models/project.rb b/app/models/project.rb index 412c6c6732d..ab4afd4159e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -404,6 +404,18 @@ class Project < ActiveRecord::Base self.import_data.destroy if self.import_data end + def import_url=(value) + sanitizer = Gitlab::ImportUrlSanitizer.new(value) + self[:import_url] = sanitizer.sanitized_url + create_import_data(credentials: sanitizer.credentials) + end + + def import_url + if import_data + Gitlab::ImportUrlExposer.expose(import_url: self[:import_url], credentials: import_data.credentials) + end + end + def import? external_import? || forked? 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 881af783c61..f98ec925ac4 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -1,29 +1,5 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration - class ImportUrlSanitizer - def initialize(url) - @url = URI.parse(url) - end - - def sanitized_url - @sanitized_url ||= safe_url - end - - def credentials - @credentials ||= { user: @url.user, password: @url.password } - end - - private - - def safe_url - safe_url = @url.dup - safe_url.password = nil - safe_url.user = nil - safe_url - end - - end - class FakeProjectImportData extend AttrEncrypted attr_accessor :credentials @@ -31,20 +7,30 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def up - projects_with_wrong_import_url.each do |project| - sanitizer = ImportUrlSanitizer.new(project["import_url"]) - - ActiveRecord::Base.transaction do - execute("UPDATE projects SET import_url = '#{sanitizer.sanitized_url}' WHERE id = #{project['id']}") - fake_import_data = FakeProjectImportData.new - fake_import_data.credentials = sanitizer.credentials - execute("UPDATE project_import_data SET encrypted_credentials = '#{fake_import_data.encrypted_credentials}' WHERE project_id = #{project['id']}") + projects_with_wrong_import_url.find_in_batches do |project_batch| + project_batch.each do |project| + sanitizer = Gitlab::ImportUrlSanitizer.new(project["import_url"]) + + ActiveRecord::Base.transaction do + execute("UPDATE projects SET import_url = '#{quote(sanitizer.sanitized_url)}' WHERE id = #{project['id']}") + fake_import_data = FakeProjectImportData.new + fake_import_data.credentials = sanitizer.credentials + execute("UPDATE project_import_data SET encrypted_credentials = '#{quote(fake_import_data.encrypted_credentials)}' WHERE project_id = #{project['id']}") + end end end end + def down + + end + def projects_with_wrong_import_url # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL - select_all("SELECT p.id, p.import_url from projects p WHERE p.import_url LIKE '%//%:%@%' or p.import_url like '#{"_"*40}@github.com%'") + select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE '#{"_"*40}@github.com%')") + end + + def quote(value) + ActiveRecord::Base.connection.quote(value) end end diff --git a/db/schema.rb b/db/schema.rb index 5b2f5aa3ddd..72a3ec2fb10 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -416,7 +416,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.string "state" t.integer "iid" t.integer "updated_by_id" - t.boolean "confidential", default: false + t.boolean "confidential", default: false end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -684,6 +684,8 @@ ActiveRecord::Schema.define(version: 20160316204731) do create_table "project_import_data", force: :cascade do |t| t.integer "project_id" t.text "data" + t.text "encrypted_credentials" + t.text "encrypted_credentials_iv" end create_table "projects", force: :cascade do |t| diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index a96cfa8af9d..d407be5dcf4 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -8,7 +8,7 @@ module Gitlab def initialize(project) @project = project credentials = project.import_data.credentials if import_data - @client = Client.new(credentials["github_access_token"]) + @client = Client.new(credentials[:user]) @formatter = Gitlab::ImportFormatter.new end diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index 52aba93a51d..f4221003db5 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -11,7 +11,7 @@ module Gitlab end def execute - project = ::Projects::CreateService.new( + ::Projects::CreateService.new( current_user, name: repo.name, path: repo.name, @@ -20,19 +20,9 @@ module Gitlab visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC, import_type: "github", import_source: repo.full_name, - import_url: repo.clone_url, + import_url: repo.clone_url.sub("https://", "https://#{@session_data[:github_access_token]}@"), wiki_enabled: !repo.has_wiki? # If repo has wiki we'll import it later ).execute - - create_import_data(project) - project - end - - private - - def create_import_data(project) - project.create_import_data( - credentials: { github_access_token: session_data.delete(:github_access_token) }) end end end diff --git a/lib/gitlab/github_import/wiki_formatter.rb b/lib/gitlab/github_import/wiki_formatter.rb index 8be82924107..db2c49a497a 100644 --- a/lib/gitlab/github_import/wiki_formatter.rb +++ b/lib/gitlab/github_import/wiki_formatter.rb @@ -12,9 +12,7 @@ module Gitlab end def import_url - import_url = Gitlab::ImportUrlExposer.expose(import_url: project.import_url, - credentials: project.import_data.credentials) - import_url.sub(/\.git\z/, ".wiki.git") + project.import_url.import_url.sub(/\.git\z/, ".wiki.git") end end end diff --git a/lib/gitlab/import_url_sanitizer.rb b/lib/gitlab/import_url_sanitizer.rb new file mode 100644 index 00000000000..dfbc4f8303c --- /dev/null +++ b/lib/gitlab/import_url_sanitizer.rb @@ -0,0 +1,24 @@ +module Gitlab + class ImportUrlSanitizer + def initialize(url) + @url = URI.parse(url) + end + + def sanitized_url + @sanitized_url ||= safe_url.to_s + end + + def credentials + @credentials ||= { user: @url.user, password: @url.password } + end + + private + + def safe_url + safe_url = @url.dup + safe_url.password = nil + safe_url.user = nil + safe_url + end + end +end \ No newline at end of file -- cgit v1.2.1 From 8d7d9c8daa61d58a17fea648771a1bb6c9341304 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 13:23:35 +0100 Subject: added missing column for current att_encrypted version --- .../20160302151724_add_import_credentials_to_project_import_data.rb | 1 + 1 file changed, 1 insertion(+) 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 ff3d8b466dc..dd2b3613983 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 @@ -2,5 +2,6 @@ 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 end end -- cgit v1.2.1 From 030b13944534be505dc97667ce2094ed6c588f12 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 15:11:05 +0100 Subject: more refactoring --- app/models/project.rb | 12 +++++--- ...152808_remove_wrong_import_url_from_projects.rb | 23 ++++++-------- db/schema.rb | 11 +++---- lib/gitlab/github_import/importer.rb | 13 ++++++-- lib/gitlab/github_import/wiki_formatter.rb | 2 +- lib/gitlab/import_url.rb | 36 ++++++++++++++++++++++ lib/gitlab/import_url_exposer.rb | 13 -------- lib/gitlab/import_url_sanitizer.rb | 24 --------------- .../gitlab/github_import/project_creator_spec.rb | 5 +-- .../gitlab/github_import/wiki_formatter_spec.rb | 7 +---- spec/lib/gitlab/import_url_exposer_spec.rb | 13 -------- spec/lib/gitlab/import_url_spec.rb | 21 +++++++++++++ 12 files changed, 94 insertions(+), 86 deletions(-) create mode 100644 lib/gitlab/import_url.rb delete mode 100644 lib/gitlab/import_url_exposer.rb delete mode 100644 lib/gitlab/import_url_sanitizer.rb delete mode 100644 spec/lib/gitlab/import_url_exposer_spec.rb create mode 100644 spec/lib/gitlab/import_url_spec.rb diff --git a/app/models/project.rb b/app/models/project.rb index ab4afd4159e..4e5fa8821ea 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -405,14 +405,17 @@ class Project < ActiveRecord::Base end def import_url=(value) - sanitizer = Gitlab::ImportUrlSanitizer.new(value) - self[:import_url] = sanitizer.sanitized_url - create_import_data(credentials: sanitizer.credentials) + import_url = Gitlab::ImportUrl.new(value) + create_import_data(credentials: import_url.credentials) + super(import_url.sanitized_url) end def import_url if import_data - Gitlab::ImportUrlExposer.expose(import_url: self[:import_url], credentials: import_data.credentials) + import_url = Gitlab::ImportUrl.new(super, credentials: import_data.credentials) + import_url.full_url + else + super end end @@ -447,6 +450,7 @@ class Project < ActiveRecord::Base def safe_import_url result = URI.parse(self.import_url) result.password = '*****' unless result.password.nil? + result.user = '*****' unless result.user.nil? #tokens or other data may be saved as user result.to_s rescue self.import_url 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 f98ec925ac4..fd718ef3974 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -1,3 +1,6 @@ +# Loops through old importer projects that kept a token/password in the import URL +# and encrypts the credentials into a separate field in project#import_data +# #down method not supported class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration class FakeProjectImportData @@ -7,24 +10,18 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def up - projects_with_wrong_import_url.find_in_batches do |project_batch| - project_batch.each do |project| - sanitizer = Gitlab::ImportUrlSanitizer.new(project["import_url"]) + projects_with_wrong_import_url do |project| + import_url = Gitlab::ImportUrl.new(project["import_url"]) - ActiveRecord::Base.transaction do - execute("UPDATE projects SET import_url = '#{quote(sanitizer.sanitized_url)}' WHERE id = #{project['id']}") - fake_import_data = FakeProjectImportData.new - fake_import_data.credentials = sanitizer.credentials - execute("UPDATE project_import_data SET encrypted_credentials = '#{quote(fake_import_data.encrypted_credentials)}' WHERE project_id = #{project['id']}") - end + ActiveRecord::Base.transaction do + execute("UPDATE projects SET import_url = '#{quote(import_url.sanitized_url)}' WHERE id = #{project['id']}") + fake_import_data = FakeProjectImportData.new + fake_import_data.credentials = import_url.credentials + execute("UPDATE project_import_data SET encrypted_credentials = '#{quote(fake_import_data.encrypted_credentials)}' WHERE project_id = #{project['id']}") end end end - def down - - end - def projects_with_wrong_import_url # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE '#{"_"*40}@github.com%')") diff --git a/db/schema.rb b/db/schema.rb index 72a3ec2fb10..02300c028d4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160316204731) do +ActiveRecord::Schema.define(version: 20160302151724) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -260,7 +260,6 @@ ActiveRecord::Schema.define(version: 20160316204731) do end add_index "ci_runners", ["description"], name: "index_ci_runners_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} - add_index "ci_runners", ["token"], name: "index_ci_runners_on_token", using: :btree add_index "ci_runners", ["token"], name: "index_ci_runners_on_token_trigram", using: :gin, opclasses: {"token"=>"gin_trgm_ops"} create_table "ci_services", force: :cascade do |t| @@ -686,6 +685,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.text "data" t.text "encrypted_credentials" t.text "encrypted_credentials_iv" + t.text "encrypted_credentials_salt" end create_table "projects", force: :cascade do |t| @@ -725,7 +725,6 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.boolean "pending_delete", default: false t.boolean "public_builds", default: true, null: false t.string "main_language" - t.integer "pushes_since_gc", default: 0 end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree @@ -811,6 +810,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.string "file_name" t.string "type" t.integer "visibility_level", default: 0, null: false + t.datetime "expires_at" end add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree @@ -869,7 +869,7 @@ ActiveRecord::Schema.define(version: 20160316204731) do create_table "todos", force: :cascade do |t| t.integer "user_id", null: false t.integer "project_id", null: false - t.integer "target_id" + t.integer "target_id", null: false t.string "target_type", null: false t.integer "author_id" t.integer "action", null: false @@ -877,11 +877,9 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.datetime "created_at" t.datetime "updated_at" t.integer "note_id" - t.string "commit_id" end add_index "todos", ["author_id"], name: "index_todos_on_author_id", using: :btree - add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree add_index "todos", ["state"], name: "index_todos_on_state", using: :btree @@ -946,7 +944,6 @@ ActiveRecord::Schema.define(version: 20160316204731) do t.string "unlock_token" t.datetime "otp_grace_period_started_at" t.boolean "ldap_email", default: false, null: false - t.boolean "external", default: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index d407be5dcf4..0b1ed510229 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -7,9 +7,12 @@ module Gitlab def initialize(project) @project = project - credentials = project.import_data.credentials if import_data - @client = Client.new(credentials[:user]) - @formatter = Gitlab::ImportFormatter.new + if import_data_credentials + @client = Client.new(import_data_credentials[:user]) + @formatter = Gitlab::ImportFormatter.new + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end end def execute @@ -18,6 +21,10 @@ module Gitlab private + def import_data_credentials + @import_data_credentials ||= project.import_data.credentials if project.import_data + end + def import_issues client.list_issues(project.import_source, state: :all, sort: :created, diff --git a/lib/gitlab/github_import/wiki_formatter.rb b/lib/gitlab/github_import/wiki_formatter.rb index db2c49a497a..6c592ff469c 100644 --- a/lib/gitlab/github_import/wiki_formatter.rb +++ b/lib/gitlab/github_import/wiki_formatter.rb @@ -12,7 +12,7 @@ module Gitlab end def import_url - project.import_url.import_url.sub(/\.git\z/, ".wiki.git") + project.import_url.sub(/\.git\z/, ".wiki.git") end end end diff --git a/lib/gitlab/import_url.rb b/lib/gitlab/import_url.rb new file mode 100644 index 00000000000..7358edac2ee --- /dev/null +++ b/lib/gitlab/import_url.rb @@ -0,0 +1,36 @@ +module Gitlab + class ImportUrl + def initialize(url, credentials: nil) + @url = URI.parse(url) + @credentials = credentials + end + + def sanitized_url + @sanitized_url ||= safe_url.to_s + end + + def credentials + @credentials ||= { user: @url.user, password: @url.password } + end + + def full_url + @full_url ||= generate_full_url.to_s + end + + private + + def generate_full_url + @full_url = @url.dup + @full_url.user = @credentials[:user] + @full_url.password = @credentials[:password] + @full_url + end + + def safe_url + safe_url = @url.dup + safe_url.password = nil + safe_url.user = nil + safe_url + end + end +end \ No newline at end of file diff --git a/lib/gitlab/import_url_exposer.rb b/lib/gitlab/import_url_exposer.rb deleted file mode 100644 index bf03f5a6daf..00000000000 --- a/lib/gitlab/import_url_exposer.rb +++ /dev/null @@ -1,13 +0,0 @@ -module Gitlab - # Exposes an import URL that includes the credentials unencrypted. - # Extracted to its own class to prevent unintended use. - module ImportUrlExposer - - def self.expose(import_url:, credentials: ) - uri = URI.parse(import_url) - uri.user = credentials[:user] - uri.password = credentials[:password] - uri - end - end -end diff --git a/lib/gitlab/import_url_sanitizer.rb b/lib/gitlab/import_url_sanitizer.rb deleted file mode 100644 index dfbc4f8303c..00000000000 --- a/lib/gitlab/import_url_sanitizer.rb +++ /dev/null @@ -1,24 +0,0 @@ -module Gitlab - class ImportUrlSanitizer - def initialize(url) - @url = URI.parse(url) - end - - def sanitized_url - @sanitized_url ||= safe_url.to_s - end - - def credentials - @credentials ||= { user: @url.user, password: @url.password } - end - - private - - def safe_url - safe_url = @url.dup - safe_url.password = nil - safe_url.user = nil - safe_url - end - end -end \ No newline at end of file diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index 290c855642a..2092c1b9584 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -26,8 +26,9 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do project_creator = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, user, access_params) project = project_creator.execute - expect(project.import_url).to eq("https://gitlab.com/asd/vim.git") - expect(project.import_data.credentials).to eq(:github_access_token => "asdffg") + expect(project.import_url).to eq("https://asdffg@gitlab.com/asd/vim.git") + expect(project.safe_import_url).to eq("https://*****@gitlab.com/asd/vim.git") + expect(project.import_data.credentials).to eq(:user => "asdffg", :password => nil) expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb index 46d5c2f3296..91cd370987d 100644 --- a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -3,12 +3,7 @@ require 'spec_helper' describe Gitlab::GithubImport::WikiFormatter, lib: true do let(:project) do create(:project, namespace: create(:namespace, path: 'gitlabhq'), - import_url: 'https://github.com/gitlabhq/sample.gitlabhq.git') - end - - let!(:project_import_data) do - create(:project_import_data, credentials: { github_access_token: 'xxx' }, - project: project) + import_url: 'https://xxx@github.com/gitlabhq/sample.gitlabhq.git') end subject(:wiki) { described_class.new(project) } diff --git a/spec/lib/gitlab/import_url_exposer_spec.rb b/spec/lib/gitlab/import_url_exposer_spec.rb deleted file mode 100644 index 878947caea1..00000000000 --- a/spec/lib/gitlab/import_url_exposer_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'spec_helper' - -describe 'Gitlab::ImportUrlExposer' do - - describe :expose do - let(:credentials) do - Gitlab::ImportUrlExposer.expose(import_url: "https://github.com/me/project.git", credentials: {user: 'blah', password: 'password'}) - end - - it { expect(credentials).to be_a(URI) } - it { expect(credentials.to_s).to eq("https://blah:password@github.com/me/project.git") } - end -end diff --git a/spec/lib/gitlab/import_url_spec.rb b/spec/lib/gitlab/import_url_spec.rb new file mode 100644 index 00000000000..f758cb8693c --- /dev/null +++ b/spec/lib/gitlab/import_url_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::ImportUrl do + + let(:credentials) { { user: 'blah', password: 'password' } } + let(:import_url) do + Gitlab::ImportUrl.new("https://github.com/me/project.git", credentials: credentials) + end + + describe :full_url do + it { expect(import_url.full_url).to eq("https://blah:password@github.com/me/project.git") } + end + + describe :sanitized_url do + it { expect(import_url.sanitized_url).to eq("https://github.com/me/project.git") } + end + + describe :credentials do + it { expect(import_url.credentials).to eq(credentials) } + end +end -- cgit v1.2.1 From bd8a77674f7226aafa4bd9befa8ed7482c372dc1 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 17:16:27 +0100 Subject: fixed few issues with the migration --- app/models/project.rb | 3 ++- app/models/project_import_data.rb | 2 +- ...152808_remove_wrong_import_url_from_projects.rb | 26 ++++++++++++++++++---- lib/gitlab/import_url.rb | 1 + 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 4e5fa8821ea..242ad19b115 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -406,6 +406,7 @@ class Project < ActiveRecord::Base def import_url=(value) import_url = Gitlab::ImportUrl.new(value) + # deletes any existing import_data create_import_data(credentials: import_url.credentials) super(import_url.sanitized_url) end @@ -450,7 +451,7 @@ class Project < ActiveRecord::Base def safe_import_url result = URI.parse(self.import_url) result.password = '*****' unless result.password.nil? - result.user = '*****' unless result.user.nil? #tokens or other data may be saved as user + result.user = '*****' unless result.user.nil? || result.user == "git" #tokens or other data may be saved as user result.to_s rescue self.import_url diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index f3b9daa0d1a..420c01f9960 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -12,7 +12,7 @@ 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 + attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true, :mode => :per_attribute_iv_and_salt serialize :data, JSON 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 fd718ef3974..0f7da3103b8 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -6,25 +6,43 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration class FakeProjectImportData extend AttrEncrypted attr_accessor :credentials - attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true + attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true, :mode => :per_attribute_iv_and_salt end def up + byebug projects_with_wrong_import_url do |project| import_url = Gitlab::ImportUrl.new(project["import_url"]) ActiveRecord::Base.transaction do - execute("UPDATE projects SET import_url = '#{quote(import_url.sanitized_url)}' WHERE id = #{project['id']}") + execute("UPDATE projects SET import_url = #{quote(import_url.sanitized_url)} WHERE id = #{project['id']}") fake_import_data = FakeProjectImportData.new fake_import_data.credentials = import_url.credentials - execute("UPDATE project_import_data SET encrypted_credentials = '#{quote(fake_import_data.encrypted_credentials)}' WHERE project_id = #{project['id']}") + project_import_data = project_import_data(project['id']) + if project_import_data + execute(update_import_data_sql(project_import_data['id'], fake_import_data)) + else + execute(insert_import_data_sql(project['id'], fake_import_data)) + end end end end + def insert_import_data_sql(project_id, fake_import_data) + %( INSERT into project_import_data (encrypted_credentials, project_id, encrypted_credentials_iv, encrypted_credentials_salt) VALUES ( #{quote(fake_import_data.encrypted_credentials)}, '#{project_id}', #{quote(fake_import_data.encrypted_credentials_iv)}, #{quote(fake_import_data.encrypted_credentials_salt)})) + end + + def update_import_data_sql(id, fake_import_data) + %( UPDATE project_import_data SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)} WHERE id = '#{id}') + end + def projects_with_wrong_import_url # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL - select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE '#{"_"*40}@github.com%')") + select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE 'https___#{"_"*40}@github.com%')") + end + + def project_import_data(project_id) + select_one("SELECT id FROM project_import_data WHERE project_id = '#{project_id}'") end def quote(value) diff --git a/lib/gitlab/import_url.rb b/lib/gitlab/import_url.rb index 7358edac2ee..aa430920252 100644 --- a/lib/gitlab/import_url.rb +++ b/lib/gitlab/import_url.rb @@ -20,6 +20,7 @@ module Gitlab private def generate_full_url + return @url unless @credentials @full_url = @url.dup @full_url.user = @credentials[:user] @full_url.password = @credentials[:password] -- cgit v1.2.1 From dff4050f1d3f00815c095ec2645bd935f14e51a7 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 17:29:19 +0100 Subject: fixed some rubocop warnings --- app/models/project_import_data.rb | 2 +- lib/gitlab/import_url.rb | 2 +- spec/factories/project_import_data.rb | 2 +- spec/lib/gitlab/github_import/project_creator_spec.rb | 2 +- spec/lib/gitlab/github_import/wiki_formatter_spec.rb | 3 ++- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 420c01f9960..ba4334055d6 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -12,7 +12,7 @@ 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 diff --git a/lib/gitlab/import_url.rb b/lib/gitlab/import_url.rb index aa430920252..adcbcc43d24 100644 --- a/lib/gitlab/import_url.rb +++ b/lib/gitlab/import_url.rb @@ -34,4 +34,4 @@ module Gitlab safe_url end end -end \ No newline at end of file +end diff --git a/spec/factories/project_import_data.rb b/spec/factories/project_import_data.rb index 18393cdda98..a799af9996c 100644 --- a/spec/factories/project_import_data.rb +++ b/spec/factories/project_import_data.rb @@ -2,4 +2,4 @@ FactoryGirl.define do factory :project_import_data, class: ProjectImportData do data "test" end -end \ No newline at end of file +end diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index 2092c1b9584..0f363b8b0aa 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -28,7 +28,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do expect(project.import_url).to eq("https://asdffg@gitlab.com/asd/vim.git") expect(project.safe_import_url).to eq("https://*****@gitlab.com/asd/vim.git") - expect(project.import_data.credentials).to eq(:user => "asdffg", :password => nil) + expect(project.import_data.credentials).to eq(user: "asdffg", password: nil) expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb index 91cd370987d..1bd29b8a563 100644 --- a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::GithubImport::WikiFormatter, lib: true do let(:project) do - create(:project, namespace: create(:namespace, path: 'gitlabhq'), + create(:project, + namespace: create(:namespace, path: 'gitlabhq'), import_url: 'https://xxx@github.com/gitlabhq/sample.gitlabhq.git') end -- cgit v1.2.1 From 6dfb5d7cad3e3cefd766e30a4c25a9549fa2b041 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 17:34:20 +0100 Subject: remove byebug --- db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb | 1 - 1 file changed, 1 deletion(-) 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 0f7da3103b8..6b4b6726fc5 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -10,7 +10,6 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def up - byebug projects_with_wrong_import_url do |project| import_url = Gitlab::ImportUrl.new(project["import_url"]) -- cgit v1.2.1 From ced56641bfc42c8380af1760fae93ba37bf2e785 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 18:09:47 +0100 Subject: refactored code based on feedback --- app/models/project.rb | 10 ++++++++-- lib/gitlab/import_url.rb | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 242ad19b115..a1aa1d5fdbc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -406,8 +406,7 @@ class Project < ActiveRecord::Base def import_url=(value) import_url = Gitlab::ImportUrl.new(value) - # deletes any existing import_data - create_import_data(credentials: import_url.credentials) + create_or_update_import_data(import_url.credentials) super(import_url.sanitized_url) end @@ -420,6 +419,13 @@ class Project < ActiveRecord::Base end end + def create_or_update_import_data(credentials) + project_import_data = import_data || ProjectImportData.new + project_import_data.credentials = credentials + project_import_data.project_id = id + project_import_data.save + end + def import? external_import? || forked? end diff --git a/lib/gitlab/import_url.rb b/lib/gitlab/import_url.rb index adcbcc43d24..5b18d67ddc3 100644 --- a/lib/gitlab/import_url.rb +++ b/lib/gitlab/import_url.rb @@ -20,10 +20,10 @@ module Gitlab private def generate_full_url - return @url unless @credentials + return @url unless credentials @full_url = @url.dup - @full_url.user = @credentials[:user] - @full_url.password = @credentials[:password] + @full_url.user = credentials[:user] + @full_url.password = credentials[:password] @full_url end -- cgit v1.2.1 From 19bfdcf4689e9c994f5a630da9cff75e5baa0e55 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 18:20:25 +0100 Subject: fix import data creation --- app/models/project.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index a1aa1d5fdbc..2a530715a9e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -420,9 +420,8 @@ class Project < ActiveRecord::Base end def create_or_update_import_data(credentials) - project_import_data = import_data || ProjectImportData.new + project_import_data = import_data || build_import_data project_import_data.credentials = credentials - project_import_data.project_id = id project_import_data.save end -- cgit v1.2.1 From 868e4918f93021430162ec097d3362f313a4bb80 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 18:38:06 +0100 Subject: updated migration --- .../20160302152808_remove_wrong_import_url_from_projects.rb | 3 +-- db/schema.rb | 10 +++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) 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 6b4b6726fc5..cf6b983323e 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -36,8 +36,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def projects_with_wrong_import_url - # TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL - select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE 'https___#{"_"*40}@github.com%')") + select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND p.import_type = 'github' AND p.import_url LIKE '%//%:%@%'") end def project_import_data(project_id) diff --git a/db/schema.rb b/db/schema.rb index 02300c028d4..71f1e1e496e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160302151724) do +ActiveRecord::Schema.define(version: 20160316204731) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -260,6 +260,7 @@ ActiveRecord::Schema.define(version: 20160302151724) do end add_index "ci_runners", ["description"], name: "index_ci_runners_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} + add_index "ci_runners", ["token"], name: "index_ci_runners_on_token", using: :btree add_index "ci_runners", ["token"], name: "index_ci_runners_on_token_trigram", using: :gin, opclasses: {"token"=>"gin_trgm_ops"} create_table "ci_services", force: :cascade do |t| @@ -725,6 +726,7 @@ ActiveRecord::Schema.define(version: 20160302151724) do t.boolean "pending_delete", default: false t.boolean "public_builds", default: true, null: false t.string "main_language" + t.integer "pushes_since_gc", default: 0 end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree @@ -810,7 +812,6 @@ ActiveRecord::Schema.define(version: 20160302151724) do t.string "file_name" t.string "type" t.integer "visibility_level", default: 0, null: false - t.datetime "expires_at" end add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree @@ -869,7 +870,7 @@ ActiveRecord::Schema.define(version: 20160302151724) do create_table "todos", force: :cascade do |t| t.integer "user_id", null: false t.integer "project_id", null: false - t.integer "target_id", null: false + t.integer "target_id" t.string "target_type", null: false t.integer "author_id" t.integer "action", null: false @@ -877,9 +878,11 @@ ActiveRecord::Schema.define(version: 20160302151724) do t.datetime "created_at" t.datetime "updated_at" t.integer "note_id" + t.string "commit_id" end add_index "todos", ["author_id"], name: "index_todos_on_author_id", using: :btree + add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree add_index "todos", ["state"], name: "index_todos_on_state", using: :btree @@ -944,6 +947,7 @@ ActiveRecord::Schema.define(version: 20160302151724) do t.string "unlock_token" t.datetime "otp_grace_period_started_at" t.boolean "ldap_email", default: false, null: false + t.boolean "external", default: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree -- cgit v1.2.1 From 1b8d995492baca8984bde950e0449dad6342befc Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Mar 2016 11:32:15 +0100 Subject: refactoring migration to add bitbucket stuff --- ...152808_remove_wrong_import_url_from_projects.rb | 55 ++++++++++++++++++---- 1 file changed, 45 insertions(+), 10 deletions(-) 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 cf6b983323e..8ba68e61c74 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -10,23 +10,52 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def up + process_projects_with_wrong_url + process_bitbucket_projects + end + + def process_projects_with_wrong_url projects_with_wrong_import_url do |project| import_url = Gitlab::ImportUrl.new(project["import_url"]) ActiveRecord::Base.transaction do - execute("UPDATE projects SET import_url = #{quote(import_url.sanitized_url)} WHERE id = #{project['id']}") - fake_import_data = FakeProjectImportData.new - fake_import_data.credentials = import_url.credentials - project_import_data = project_import_data(project['id']) - if project_import_data - execute(update_import_data_sql(project_import_data['id'], fake_import_data)) - else - execute(insert_import_data_sql(project['id'], fake_import_data)) - end + update_import_url(import_url, project) + update_import_data(import_url, project) + end + end + end + + def process_bitbucket_projects + bitbucket_projects_with_wrong_import_url do |bitbucket_data| + data = bitbucket_data['data'] + data_hash = YAML::load(data) + if data_hash && data_hash['bb_session'] + update_import_data_for_bitbucket(data_hash, bitbucket_data['id']) end end end + def update_import_data(import_url, project) + fake_import_data = FakeProjectImportData.new + fake_import_data.credentials = import_url.credentials + project_import_data = project_import_data(project['id']) + if project_import_data + execute(update_import_data_sql(project_import_data['id'], fake_import_data)) + else + execute(insert_import_data_sql(project['id'], fake_import_data)) + end + end + + def update_import_data_for_bitbucket(data_hash, import_data_id) + fake_import_data = FakeProjectImportData.new + fake_import_data.credentials = data_hash + execute(update_import_data_sql(import_data_id, fake_import_data)) + end + + def update_import_url(import_url, project) + execute("UPDATE projects SET import_url = #{quote(import_url.sanitized_url)} WHERE id = #{project['id']}") + end + def insert_import_data_sql(project_id, fake_import_data) %( INSERT into project_import_data (encrypted_credentials, project_id, encrypted_credentials_iv, encrypted_credentials_salt) VALUES ( #{quote(fake_import_data.encrypted_credentials)}, '#{project_id}', #{quote(fake_import_data.encrypted_credentials_iv)}, #{quote(fake_import_data.encrypted_credentials_salt)})) end @@ -35,8 +64,14 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration %( UPDATE project_import_data SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)} WHERE id = '#{id}') end + #Github projects with token, and any user:password@ based URL def projects_with_wrong_import_url - select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND p.import_type = 'github' AND p.import_url LIKE '%//%:%@%'") + select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE 'https___#{"_"*40}@github.com%')") + end + + # All bitbucket imports + def bitbucket_projects_with_wrong_import_url + select_all("SELECT p.id, p.import_urlselect_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = 'bitbucket' "), i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = 'bitbucket' ") end def project_import_data(project_id) -- cgit v1.2.1 From 23146fca1878c9f6e90deed6856d3da2c731d513 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Mar 2016 12:25:28 +0100 Subject: update bitbucket importer --- ...60302152808_remove_wrong_import_url_from_projects.rb | 2 +- lib/gitlab/bitbucket_import/importer.rb | 17 ++++++++++++----- lib/gitlab/github_import/importer.rb | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) 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 8ba68e61c74..4ca245035d3 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -71,7 +71,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration # All bitbucket imports def bitbucket_projects_with_wrong_import_url - select_all("SELECT p.id, p.import_urlselect_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = 'bitbucket' "), i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = 'bitbucket' ") + select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = 'bitbucket' ") end def project_import_data(project_id) diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 46e51a4bf6d..36110962e0c 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -5,11 +5,14 @@ module Gitlab def initialize(project) @project = project - import_data = project.import_data.try(:data) - bb_session = import_data["bb_session"] if import_data - @client = Client.new(bb_session["bitbucket_access_token"], - bb_session["bitbucket_access_token_secret"]) - @formatter = Gitlab::ImportFormatter.new + if import_data_credentials && import_data_credentials['bb_session'] + token = import_data_credentials['bb_session']['bitbucket_access_token'] + token_secret = import_data_credentials['bb_session']['bitbucket_access_token_secret'] + @client = Client.new(token, token_secret) + @formatter = Gitlab::ImportFormatter.new + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end end def execute @@ -24,6 +27,10 @@ module Gitlab private + def import_data_credentials + @import_data_credentials ||= project.import_data.credentials if project.import_data + end + def gl_user_id(project, bitbucket_id) if bitbucket_id user = User.joins(:identities).find_by("identities.extern_uid = ? AND identities.provider = 'bitbucket'", bitbucket_id.to_s) diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 0b1ed510229..a5d3ab5fcf1 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -8,7 +8,7 @@ module Gitlab def initialize(project) @project = project if import_data_credentials - @client = Client.new(import_data_credentials[:user]) + @client = Client.new(import_data_credentials['user']) @formatter = Gitlab::ImportFormatter.new else raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" -- cgit v1.2.1 From 3b78885eac025b5cf90e1a370b7bbbbd88cc9727 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Mar 2016 12:26:50 +0100 Subject: encrypt credentials in project_creator for bitbucket by default --- lib/gitlab/bitbucket_import/project_creator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 03aac1a025a..c9ccecef0c3 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -23,7 +23,7 @@ module Gitlab import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git", ).execute - project.create_import_data(data: { "bb_session" => session_data } ) + project.create_import_data(credentials: { "bb_session" => session_data } ) project end end -- cgit v1.2.1 From 8aafe685837d12b623f70eec86cae6e7cef9a849 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Mar 2016 17:53:53 +0100 Subject: first round of fixes and spec fixes --- app/models/project.rb | 5 +++-- lib/gitlab/bitbucket_import/importer.rb | 19 +--------------- lib/gitlab/bitbucket_import/importer_init.rb | 27 +++++++++++++++++++++++ lib/gitlab/bitbucket_import/key_deleter.rb | 13 ++--------- lib/gitlab/bitbucket_import/project_creator.rb | 4 ++-- spec/lib/gitlab/bitbucket_import/importer_spec.rb | 8 +++---- 6 files changed, 39 insertions(+), 37 deletions(-) create mode 100644 lib/gitlab/bitbucket_import/importer_init.rb diff --git a/app/models/project.rb b/app/models/project.rb index c4287d314ea..b4643563260 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -407,7 +407,7 @@ class Project < ActiveRecord::Base end def import_url - if import_data + if import_data && super import_url = Gitlab::ImportUrl.new(super, credentials: import_data.credentials) import_url.full_url else @@ -417,7 +417,8 @@ class Project < ActiveRecord::Base def create_or_update_import_data(credentials) project_import_data = import_data || build_import_data - project_import_data.credentials = credentials + project_import_data.credentials ||= {} + project_import_data.credentials.merge!(credentials) project_import_data.save end diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 36110962e0c..f80410641a9 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -1,19 +1,6 @@ module Gitlab module BitbucketImport - class Importer - attr_reader :project, :client - - def initialize(project) - @project = project - if import_data_credentials && import_data_credentials['bb_session'] - token = import_data_credentials['bb_session']['bitbucket_access_token'] - token_secret = import_data_credentials['bb_session']['bitbucket_access_token_secret'] - @client = Client.new(token, token_secret) - @formatter = Gitlab::ImportFormatter.new - else - raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" - end - end + class Importer < ImporterInit def execute import_issues if has_issues? @@ -27,10 +14,6 @@ module Gitlab private - def import_data_credentials - @import_data_credentials ||= project.import_data.credentials if project.import_data - end - def gl_user_id(project, bitbucket_id) if bitbucket_id user = User.joins(:identities).find_by("identities.extern_uid = ? AND identities.provider = 'bitbucket'", bitbucket_id.to_s) diff --git a/lib/gitlab/bitbucket_import/importer_init.rb b/lib/gitlab/bitbucket_import/importer_init.rb new file mode 100644 index 00000000000..6d81811d36b --- /dev/null +++ b/lib/gitlab/bitbucket_import/importer_init.rb @@ -0,0 +1,27 @@ +module Gitlab + module BitbucketImport + class ImporterInit + attr_reader :project, :client + + def initialize(project) + @project = project + if import_data_credentials && import_data_credentials['bb_session'] + token = import_data_credentials['bb_session']['bitbucket_access_token'] + token_secret = import_data_credentials['bb_session']['bitbucket_access_token_secret'] + @client = Client.new(token, token_secret) + @formatter = Gitlab::ImportFormatter.new + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end + end + + private + + def import_data_credentials + @import_data_credentials ||= project.import_data.credentials if project.import_data + end + end + end +end + + diff --git a/lib/gitlab/bitbucket_import/key_deleter.rb b/lib/gitlab/bitbucket_import/key_deleter.rb index f4dd393ad29..312ed581500 100644 --- a/lib/gitlab/bitbucket_import/key_deleter.rb +++ b/lib/gitlab/bitbucket_import/key_deleter.rb @@ -1,16 +1,7 @@ module Gitlab module BitbucketImport - class KeyDeleter - attr_reader :project, :current_user, :client - - def initialize(project) - @project = project - @current_user = project.creator - import_data = project.import_data.try(:data) - bb_session = import_data["bb_session"] if import_data - @client = Client.new(bb_session["bitbucket_access_token"], - bb_session["bitbucket_access_token_secret"]) - end + class KeyDeleter < ImporterInit + attr_reader :current_user def execute return false unless BitbucketImport.public_key.present? diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index c9ccecef0c3..0e6f66de321 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -22,8 +22,8 @@ module Gitlab import_source: "#{repo["owner"]}/#{repo["slug"]}", import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git", ).execute - - project.create_import_data(credentials: { "bb_session" => session_data } ) + import_data = project.import_data + import_data.credentials.merge!("bb_session" => session_data) project end end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index c413132abe5..1a833f255a5 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -34,9 +34,9 @@ describe Gitlab::BitbucketImport::Importer, lib: true do let(:project_identifier) { 'namespace/repo' } let(:data) do { - bb_session: { - bitbucket_access_token: "123456", - bitbucket_access_token_secret: "secret" + 'bb_session' => { + 'bitbucket_access_token' => "123456", + 'bitbucket_access_token_secret' => "secret" } } end @@ -44,7 +44,7 @@ describe Gitlab::BitbucketImport::Importer, lib: true do create( :project, import_source: project_identifier, - import_data: ProjectImportData.new(data: data) + import_data: ProjectImportData.new(credentials: data) ) end let(:importer) { Gitlab::BitbucketImport::Importer.new(project) } -- cgit v1.2.1 From c136edbbe343cbb54f135928db5d901c1cb65c4f Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Mar 2016 18:03:54 +0100 Subject: fix gitlab import and spec --- lib/gitlab/gitlab_import/importer.rb | 11 +++++++---- lib/gitlab/gitlab_import/project_creator.rb | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index 850b73244c6..afc06d01ebd 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -5,10 +5,13 @@ module Gitlab def initialize(project) @project = project - import_data = project.import_data.try(:data) - gitlab_session = import_data["gitlab_session"] if import_data - @client = Client.new(gitlab_session["gitlab_access_token"]) - @formatter = Gitlab::ImportFormatter.new + credentials = import_data.credentials + if credentials && credentials["password"] + @client = Client.new(credentials["password"]) + @formatter = Gitlab::ImportFormatter.new + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end end def execute diff --git a/lib/gitlab/gitlab_import/project_creator.rb b/lib/gitlab/gitlab_import/project_creator.rb index 7baaadb813c..77c33db4b59 100644 --- a/lib/gitlab/gitlab_import/project_creator.rb +++ b/lib/gitlab/gitlab_import/project_creator.rb @@ -23,7 +23,6 @@ module Gitlab import_url: repo["http_url_to_repo"].sub("://", "://oauth2:#{@session_data[:gitlab_access_token]}@") ).execute - project.create_import_data(data: { "gitlab_session" => session_data } ) project end end -- cgit v1.2.1 From 46346caf5ba5af7ad9af6913e479f301a3ff2d2d Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 22 Mar 2016 18:14:13 +0100 Subject: fix rubocop warning --- lib/gitlab/bitbucket_import/importer_init.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/gitlab/bitbucket_import/importer_init.rb b/lib/gitlab/bitbucket_import/importer_init.rb index 6d81811d36b..08b710003e4 100644 --- a/lib/gitlab/bitbucket_import/importer_init.rb +++ b/lib/gitlab/bitbucket_import/importer_init.rb @@ -23,5 +23,3 @@ module Gitlab end end end - - -- cgit v1.2.1 From 8da04f6b64884c642e8e4b630a30e6400e47b09e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 23 Mar 2016 11:55:43 +0100 Subject: refactored migration a bit and fixed a few problems. Also added some logging --- ...152808_remove_wrong_import_url_from_projects.rb | 36 ++++++++++++++-------- 1 file changed, 23 insertions(+), 13 deletions(-) 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 4ca245035d3..3d97a66c0ff 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -10,31 +10,41 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def up - process_projects_with_wrong_url - process_bitbucket_projects + say("Encrypting and migrating project import credentials...") + + say("Projects and Github projects with a wrong URL") + in_transaction { process_projects_with_wrong_url } + + say("Migrating bitbucket credentials...") + in_transaction { process_bitbucket_projects } end def process_projects_with_wrong_url - projects_with_wrong_import_url do |project| + projects_with_wrong_import_url.each do |project| import_url = Gitlab::ImportUrl.new(project["import_url"]) - ActiveRecord::Base.transaction do - update_import_url(import_url, project) - update_import_data(import_url, project) - end + update_import_url(import_url, project) + update_import_data(import_url, project) end end def process_bitbucket_projects - bitbucket_projects_with_wrong_import_url do |bitbucket_data| - data = bitbucket_data['data'] - data_hash = YAML::load(data) - if data_hash && data_hash['bb_session'] + bitbucket_projects_with_wrong_import_url.each do |bitbucket_data| + data_hash = YAML::load(bitbucket_data['data']) if bitbucket_data['data'] + if defined?(data_hash) && data_hash && data_hash['bb_session'] update_import_data_for_bitbucket(data_hash, bitbucket_data['id']) end end end + def in_transaction + say_with_time("Processing new transaction...") do + ActiveRecord::Base.transaction do + yield + end + end + end + def update_import_data(import_url, project) fake_import_data = FakeProjectImportData.new fake_import_data.credentials = import_url.credentials @@ -60,8 +70,8 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration %( INSERT into project_import_data (encrypted_credentials, project_id, encrypted_credentials_iv, encrypted_credentials_salt) VALUES ( #{quote(fake_import_data.encrypted_credentials)}, '#{project_id}', #{quote(fake_import_data.encrypted_credentials_iv)}, #{quote(fake_import_data.encrypted_credentials_salt)})) end - def update_import_data_sql(id, fake_import_data) - %( UPDATE project_import_data SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)} WHERE id = '#{id}') + def update_import_data_sql(id, fake_import_data, data = 'NULL') + %( UPDATE project_import_data SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)}, data = #{data} WHERE id = '#{id}') end #Github projects with token, and any user:password@ based URL -- cgit v1.2.1 From 6967871fc567cbadd63f26f1ef87c4008cc6387b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 23 Mar 2016 12:52:50 +0100 Subject: fogbugz importer, also refactored migration again to make it easier to add new importers --- ...152808_remove_wrong_import_url_from_projects.rb | 31 +++++++++++++--------- lib/gitlab/fogbugz_import/importer.rb | 13 ++++++--- lib/gitlab/fogbugz_import/project_creator.rb | 2 +- 3 files changed, 30 insertions(+), 16 deletions(-) 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 3d97a66c0ff..f3a4fc26be9 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -12,11 +12,14 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def up say("Encrypting and migrating project import credentials...") - say("Projects and Github projects with a wrong URL") + say("Projects and Github projects with a wrong URL. Also, migrating Gitlab projects credentials.") in_transaction { process_projects_with_wrong_url } say("Migrating bitbucket credentials...") - in_transaction { process_bitbucket_projects } + in_transaction { process_project(import_type: 'bitbucket') } + + say("Migrating fogbugz credentials...") + in_transaction { process_project(import_type: 'fogbugz') } end def process_projects_with_wrong_url @@ -28,12 +31,16 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end end - def process_bitbucket_projects - bitbucket_projects_with_wrong_import_url.each do |bitbucket_data| - data_hash = YAML::load(bitbucket_data['data']) if bitbucket_data['data'] - if defined?(data_hash) && data_hash && data_hash['bb_session'] - update_import_data_for_bitbucket(data_hash, bitbucket_data['id']) - end + def process_project(import_type: ) + unencrypted_import_data(import_type: import_type).each do |data| + replace_data_credentials(data) + end + end + + def replace_data_credentials(data) + data_hash = YAML::load(data['data']) if data['data'] + if defined?(data_hash) && data_hash + update_with_encrypted_data(data_hash, data['id']) end end @@ -56,7 +63,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end end - def update_import_data_for_bitbucket(data_hash, import_data_id) + def update_with_encrypted_data(data_hash, import_data_id) fake_import_data = FakeProjectImportData.new fake_import_data.credentials = data_hash execute(update_import_data_sql(import_data_id, fake_import_data)) @@ -79,9 +86,9 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE 'https___#{"_"*40}@github.com%')") end - # All bitbucket imports - def bitbucket_projects_with_wrong_import_url - select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = 'bitbucket' ") + # All imports with data for import_type + def unencrypted_import_data(import_type: ) + select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = '#{import_type}' ") end def project_import_data(project_id) diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index db580b5e578..c33b3541dd8 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -8,9 +8,12 @@ module Gitlab import_data = project.import_data.try(:data) repo_data = import_data['repo'] if import_data - @repo = FogbugzImport::Repository.new(repo_data) - - @known_labels = Set.new + if import_data_credentials && import_data_credentials['repo'] + @repo = FogbugzImport::Repository.new(repo_data) + @known_labels = Set.new + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end end def execute @@ -30,6 +33,10 @@ module Gitlab private + def import_data_credentials + @import_data_credentials ||= project.import_data.credentials if project.import_data + end + def user_map @user_map ||= begin user_map = Hash.new diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index e0163499e30..73d6272720a 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -25,7 +25,7 @@ module Gitlab ).execute project.create_import_data( - data: { + credentials: { 'repo' => repo.raw_data, 'user_map' => user_map, 'fb_session' => fb_session -- cgit v1.2.1 From cc4d04f97f891479c4d033196c6868e19528c51c Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 23 Mar 2016 17:57:10 +0100 Subject: added rest of importers, fixed specs and some issues with the migration --- ...02152808_remove_wrong_import_url_from_projects.rb | 10 +++++++--- db/schema.rb | 4 ++-- lib/gitlab/google_code_import/importer.rb | 20 ++++++++++++-------- lib/gitlab/google_code_import/project_creator.rb | 2 +- spec/lib/gitlab/google_code_import/importer_spec.rb | 2 +- 5 files changed, 23 insertions(+), 15 deletions(-) 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 f3a4fc26be9..a55ab6a42b4 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -12,7 +12,8 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def up say("Encrypting and migrating project import credentials...") - say("Projects and Github projects with a wrong URL. Also, migrating Gitlab projects 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 } say("Migrating bitbucket credentials...") @@ -20,6 +21,9 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration say("Migrating fogbugz credentials...") in_transaction { process_project(import_type: 'fogbugz') } + + say("Migrating google code credentials...") + in_transaction { process_project(import_type: 'google_code') } end def process_projects_with_wrong_url @@ -39,7 +43,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def replace_data_credentials(data) data_hash = YAML::load(data['data']) if data['data'] - if defined?(data_hash) && data_hash + if defined?(data_hash) && !data_hash.blank? update_with_encrypted_data(data_hash, data['id']) end end @@ -83,7 +87,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration #Github projects with token, and any user:password@ based URL def projects_with_wrong_import_url - select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND (p.import_url LIKE '%//%:%@%' OR p.import_url LIKE 'https___#{"_"*40}@github.com%')") + select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") end # All imports with data for import_type diff --git a/db/schema.rb b/db/schema.rb index 53d12aa35dd..75509c35b05 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -417,9 +417,9 @@ ActiveRecord::Schema.define(version: 20160320204112) do t.string "state" t.integer "iid" t.integer "updated_by_id" - t.integer "moved_to_id" - t.boolean "confidential", default: false + t.boolean "confidential", default: false t.datetime "deleted_at" + t.integer "moved_to_id" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 62da327931f..6b0715d1492 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -6,12 +6,13 @@ module Gitlab def initialize(project) @project = project - import_data = project.import_data.try(:data) - repo_data = import_data["repo"] if import_data - @repo = GoogleCodeImport::Repository.new(repo_data) - - @closed_statuses = [] - @known_labels = Set.new + if import_data_credentials && import_data_credentials['repo'] + @repo = GoogleCodeImport::Repository.new(import_data_credentials['repo']) + @closed_statuses = [] + @known_labels = Set.new + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end end def execute @@ -28,6 +29,10 @@ module Gitlab private + def import_data_credentials + @import_data_credentials ||= project.import_data.credentials if project.import_data + end + def user_map @user_map ||= begin user_map = Hash.new do |hash, user| @@ -35,8 +40,7 @@ module Gitlab Client.mask_email(user).sub("...", "\\.\\.\\.") end - import_data = project.import_data.try(:data) - stored_user_map = import_data["user_map"] if import_data + stored_user_map = import_data_credentials["user_map"] user_map.update(stored_user_map) if stored_user_map user_map diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb index 87821c23460..acd3a832d59 100644 --- a/lib/gitlab/google_code_import/project_creator.rb +++ b/lib/gitlab/google_code_import/project_creator.rb @@ -25,7 +25,7 @@ module Gitlab ).execute project.create_import_data( - data: { + credentials: { "repo" => repo.raw_data, "user_map" => user_map } diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb index 647631271e0..6ecf3d7182f 100644 --- a/spec/lib/gitlab/google_code_import/importer_spec.rb +++ b/spec/lib/gitlab/google_code_import/importer_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do subject { described_class.new(project) } before do - project.create_import_data(data: import_data) + project.create_import_data(credentials: import_data) end describe "#execute" do -- cgit v1.2.1 From 459ad34493c57b40fd431b18750fef85884d51e1 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 28 Mar 2016 16:35:03 +0200 Subject: refactored code based on feedback plus fixed a couple of other issues --- ...152808_remove_wrong_import_url_from_projects.rb | 66 ++++++++++++++-------- lib/gitlab/bitbucket_import/client.rb | 11 ++++ lib/gitlab/bitbucket_import/importer.rb | 9 ++- lib/gitlab/bitbucket_import/importer_init.rb | 25 -------- lib/gitlab/bitbucket_import/key_deleter.rb | 10 +++- lib/gitlab/google_code_import/importer.rb | 20 +++---- lib/gitlab/google_code_import/project_creator.rb | 2 +- .../lib/gitlab/google_code_import/importer_spec.rb | 2 +- 8 files changed, 81 insertions(+), 64 deletions(-) delete mode 100644 lib/gitlab/bitbucket_import/importer_init.rb 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 a55ab6a42b4..fb5d8591c09 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -16,14 +16,12 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration say("Projects and Github projects with a wrong URL. It also migrates Gitlab project credentials.") in_transaction { process_projects_with_wrong_url } - say("Migrating bitbucket credentials...") - in_transaction { process_project(import_type: 'bitbucket') } + say("Migrating bitbucket credentials...")# TODO remove last param + in_transaction { process_project(import_type: 'bitbucket', unencrypted_data: ['repo', 'user_map']) } say("Migrating fogbugz credentials...") - in_transaction { process_project(import_type: 'fogbugz') } + in_transaction { process_project(import_type: 'fogbugz', unencrypted_data: ['repo', 'user_map']) } - say("Migrating google code credentials...") - in_transaction { process_project(import_type: 'google_code') } end def process_projects_with_wrong_url @@ -35,19 +33,29 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end end - def process_project(import_type: ) + def process_project(import_type: , unencrypted_data: []) unencrypted_import_data(import_type: import_type).each do |data| - replace_data_credentials(data) + replace_data_credentials(data, unencrypted_data) end end - def replace_data_credentials(data) - data_hash = YAML::load(data['data']) if data['data'] + def replace_data_credentials(data, unencrypted_data) + data_hash = JSON.load(data['data']) if data['data'] if defined?(data_hash) && !data_hash.blank? - update_with_encrypted_data(data_hash, data['id']) + unencrypted_data_hash = encrypted_data_hash(data_hash, unencrypted_data) + update_with_encrypted_data(data_hash, data['id'], unencrypted_data_hash) end end + def encrypted_data_hash(data_hash, unencrypted_data) + return 'NULL' if unencrypted_data.empty? + new_data_hash = {} + unencrypted_data.each do |key| + new_data_hash[key] = data_hash.delete(key) if data_hash[key] + end + quote(new_data_hash.to_json) + end + def in_transaction say_with_time("Processing new transaction...") do ActiveRecord::Base.transaction do @@ -59,18 +67,18 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def update_import_data(import_url, project) fake_import_data = FakeProjectImportData.new fake_import_data.credentials = import_url.credentials - project_import_data = project_import_data(project['id']) - if project_import_data - execute(update_import_data_sql(project_import_data['id'], fake_import_data)) + import_data_id = project['import_data_id'] + if import_data_id + execute(update_import_data_sql(import_data_id, fake_import_data)) else execute(insert_import_data_sql(project['id'], fake_import_data)) end end - def update_with_encrypted_data(data_hash, import_data_id) + def update_with_encrypted_data(data_hash, import_data_id, data_array = nil) fake_import_data = FakeProjectImportData.new fake_import_data.credentials = data_hash - execute(update_import_data_sql(import_data_id, fake_import_data)) + execute(update_import_data_sql(import_data_id, fake_import_data, data_array)) end def update_import_url(import_url, project) @@ -78,16 +86,34 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end def insert_import_data_sql(project_id, fake_import_data) - %( INSERT into project_import_data (encrypted_credentials, project_id, encrypted_credentials_iv, encrypted_credentials_salt) VALUES ( #{quote(fake_import_data.encrypted_credentials)}, '#{project_id}', #{quote(fake_import_data.encrypted_credentials_iv)}, #{quote(fake_import_data.encrypted_credentials_salt)})) + %( + INSERT INTO project_import_data + (encrypted_credentials, + project_id, + encrypted_credentials_iv, + encrypted_credentials_salt) + VALUES ( #{quote(fake_import_data.encrypted_credentials)}, + '#{project_id}', + #{quote(fake_import_data.encrypted_credentials_iv)}, + #{quote(fake_import_data.encrypted_credentials_salt)}) + ).squish end def update_import_data_sql(id, fake_import_data, data = 'NULL') - %( UPDATE project_import_data SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)}, data = #{data} WHERE id = '#{id}') + %( + UPDATE project_import_data + SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, + encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, + encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)}, + data = #{data} + WHERE id = '#{id}' + ).squish end #Github projects with token, and any user:password@ based URL + #TODO: may need to add import_type != list def projects_with_wrong_import_url - select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") + select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.id WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") end # All imports with data for import_type @@ -95,10 +121,6 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = '#{import_type}' ") end - def project_import_data(project_id) - select_one("SELECT id FROM project_import_data WHERE project_id = '#{project_id}'") - end - def quote(value) ActiveRecord::Base.connection.quote(value) end diff --git a/lib/gitlab/bitbucket_import/client.rb b/lib/gitlab/bitbucket_import/client.rb index d88a6eaac6b..1d1bd5e3216 100644 --- a/lib/gitlab/bitbucket_import/client.rb +++ b/lib/gitlab/bitbucket_import/client.rb @@ -5,6 +5,17 @@ module Gitlab attr_reader :consumer, :api + def self.from_project(project) + credentials = project.import_data.credentials if project.import_data + if defined?(credentials) && credentials['bb_session'] + token = credentials['bb_session']['bitbucket_access_token'] + token_secret = credentials['bb_session']['bitbucket_access_token_secret'] + new(token, token_secret) + else + raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" + end + end + def initialize(access_token = nil, access_token_secret = nil) @consumer = ::OAuth::Consumer.new( config.app_id, diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index f80410641a9..7beaecd1cf0 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -1,6 +1,13 @@ module Gitlab module BitbucketImport - class Importer < ImporterInit + class Importer + attr_reader :project, :client + + def initialize(project) + @project = project + @client = Client.from_project(@project) + @formatter = Gitlab::ImportFormatter.new + end def execute import_issues if has_issues? diff --git a/lib/gitlab/bitbucket_import/importer_init.rb b/lib/gitlab/bitbucket_import/importer_init.rb deleted file mode 100644 index 08b710003e4..00000000000 --- a/lib/gitlab/bitbucket_import/importer_init.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Gitlab - module BitbucketImport - class ImporterInit - attr_reader :project, :client - - def initialize(project) - @project = project - if import_data_credentials && import_data_credentials['bb_session'] - token = import_data_credentials['bb_session']['bitbucket_access_token'] - token_secret = import_data_credentials['bb_session']['bitbucket_access_token_secret'] - @client = Client.new(token, token_secret) - @formatter = Gitlab::ImportFormatter.new - else - raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" - end - end - - private - - def import_data_credentials - @import_data_credentials ||= project.import_data.credentials if project.import_data - end - end - end -end diff --git a/lib/gitlab/bitbucket_import/key_deleter.rb b/lib/gitlab/bitbucket_import/key_deleter.rb index 312ed581500..e03c3155b3e 100644 --- a/lib/gitlab/bitbucket_import/key_deleter.rb +++ b/lib/gitlab/bitbucket_import/key_deleter.rb @@ -1,7 +1,13 @@ module Gitlab module BitbucketImport - class KeyDeleter < ImporterInit - attr_reader :current_user + class KeyDeleter + attr_reader :project, :current_user, :client + + def initialize(project) + @project = project + @current_user = project.creator + @client = Client.from_project(@project) + end def execute return false unless BitbucketImport.public_key.present? diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 6b0715d1492..62da327931f 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -6,13 +6,12 @@ module Gitlab def initialize(project) @project = project - if import_data_credentials && import_data_credentials['repo'] - @repo = GoogleCodeImport::Repository.new(import_data_credentials['repo']) - @closed_statuses = [] - @known_labels = Set.new - else - raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" - end + import_data = project.import_data.try(:data) + repo_data = import_data["repo"] if import_data + @repo = GoogleCodeImport::Repository.new(repo_data) + + @closed_statuses = [] + @known_labels = Set.new end def execute @@ -29,10 +28,6 @@ module Gitlab private - def import_data_credentials - @import_data_credentials ||= project.import_data.credentials if project.import_data - end - def user_map @user_map ||= begin user_map = Hash.new do |hash, user| @@ -40,7 +35,8 @@ module Gitlab Client.mask_email(user).sub("...", "\\.\\.\\.") end - stored_user_map = import_data_credentials["user_map"] + import_data = project.import_data.try(:data) + stored_user_map = import_data["user_map"] if import_data user_map.update(stored_user_map) if stored_user_map user_map diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb index acd3a832d59..87821c23460 100644 --- a/lib/gitlab/google_code_import/project_creator.rb +++ b/lib/gitlab/google_code_import/project_creator.rb @@ -25,7 +25,7 @@ module Gitlab ).execute project.create_import_data( - credentials: { + data: { "repo" => repo.raw_data, "user_map" => user_map } diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb index 6ecf3d7182f..647631271e0 100644 --- a/spec/lib/gitlab/google_code_import/importer_spec.rb +++ b/spec/lib/gitlab/google_code_import/importer_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do subject { described_class.new(project) } before do - project.create_import_data(credentials: import_data) + project.create_import_data(data: import_data) end describe "#execute" do -- cgit v1.2.1 From 28df200c66f88348e0c995fc8023b6dd78b9bf9b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 29 Mar 2016 15:23:32 +0200 Subject: fixed failing specs --- lib/gitlab/import_url.rb | 6 +++++- spec/factories/project_import_data.rb | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/import_url.rb b/lib/gitlab/import_url.rb index 5b18d67ddc3..3cfbc17b89b 100644 --- a/lib/gitlab/import_url.rb +++ b/lib/gitlab/import_url.rb @@ -20,7 +20,7 @@ module Gitlab private def generate_full_url - return @url unless credentials + return @url unless valid_credentials? @full_url = @url.dup @full_url.user = credentials[:user] @full_url.password = credentials[:password] @@ -33,5 +33,9 @@ module Gitlab safe_url.user = nil safe_url end + + def valid_credentials? + credentials && credentials.is_a?(Hash) && credentials.any? + end end end diff --git a/spec/factories/project_import_data.rb b/spec/factories/project_import_data.rb index a799af9996c..9e08d5a22e9 100644 --- a/spec/factories/project_import_data.rb +++ b/spec/factories/project_import_data.rb @@ -1,5 +1,6 @@ FactoryGirl.define do factory :project_import_data, class: ProjectImportData do data "test" + project end end -- cgit v1.2.1 From c93570d8fb439a8a60a204bcc41e5e8a720730e4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 29 Mar 2016 16:10:35 +0200 Subject: fixing a few issues after testing imports --- lib/gitlab/bitbucket_import/project_creator.rb | 3 +++ lib/gitlab/fogbugz_import/project_creator.rb | 11 ++++------- lib/gitlab/google_code_import/project_creator.rb | 9 +++------ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 0e6f66de321..7dc3e01753a 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -22,8 +22,11 @@ module Gitlab import_source: "#{repo["owner"]}/#{repo["slug"]}", import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git", ).execute + import_data = project.import_data import_data.credentials.merge!("bb_session" => session_data) + import_data.save + project end end diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index 73d6272720a..15914aea3b9 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -24,13 +24,10 @@ module Gitlab import_url: Project::UNKNOWN_IMPORT_URL ).execute - project.create_import_data( - credentials: { - 'repo' => repo.raw_data, - 'user_map' => user_map, - 'fb_session' => fb_session - } - ) + import_data = project.import_data + import_data.credentials.merge!('fb_session' => fb_session) + import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } + import_data.save project end diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb index 87821c23460..49d6013af28 100644 --- a/lib/gitlab/google_code_import/project_creator.rb +++ b/lib/gitlab/google_code_import/project_creator.rb @@ -24,12 +24,9 @@ module Gitlab import_url: repo.import_url ).execute - project.create_import_data( - data: { - "repo" => repo.raw_data, - "user_map" => user_map - } - ) + import_data = project.import_data + import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } + import_data.save project end -- cgit v1.2.1 From 075b56aae2e045e930580985234276edb353747f Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 29 Mar 2016 18:43:23 +0200 Subject: more fixes after doing more manual testing on importing --- lib/gitlab/bitbucket_import/project_creator.rb | 3 ++- lib/gitlab/fogbugz_import/project_creator.rb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 7dc3e01753a..cc7f2017142 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -24,7 +24,8 @@ module Gitlab ).execute import_data = project.import_data - import_data.credentials.merge!("bb_session" => session_data) + # merge! with a bang doesn't work here + import_data.credentials = import_data.credentials.merge("bb_session" => session_data) import_data.save project diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index 15914aea3b9..939cb96c101 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -25,7 +25,8 @@ module Gitlab ).execute import_data = project.import_data - import_data.credentials.merge!('fb_session' => fb_session) + # merge! with a bang doesn't work here + import_data.credentials = import_data.credentials.merge('fb_session' => fb_session) import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } import_data.save -- cgit v1.2.1 From 6d12d79d29e035c7238aa7112db1429711e61a65 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 1 Apr 2016 12:04:41 +0200 Subject: fix fogbugz import --- app/models/project_import_data.rb | 4 ++++ lib/gitlab/fogbugz_import/importer.rb | 14 ++++++-------- lib/gitlab/fogbugz_import/project_creator.rb | 3 ++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index ba4334055d6..fa6055ff64d 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -17,4 +17,8 @@ class ProjectImportData < ActiveRecord::Base serialize :data, JSON validates :project, presence: true + + def stringified_credentials + JSON[credentials.to_json] + end end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index c33b3541dd8..c88a44573a7 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -8,7 +8,7 @@ module Gitlab import_data = project.import_data.try(:data) repo_data = import_data['repo'] if import_data - if import_data_credentials && import_data_credentials['repo'] + if defined?(repo_data) @repo = FogbugzImport::Repository.new(repo_data) @known_labels = Set.new else @@ -18,10 +18,8 @@ module Gitlab def execute return true unless repo.valid? - - data = project.import_data.try(:data) - - client = Gitlab::FogbugzImport::Client.new(token: data['fb_session']['token'], uri: data['fb_session']['uri']) + Rails.logger.error import_data_credentials.inspect + client = Gitlab::FogbugzImport::Client.new(token: import_data_credentials['fb_session']['token'], uri: import_data_credentials['fb_session']['uri']) @cases = client.cases(@repo.id.to_i) @categories = client.categories @@ -34,7 +32,7 @@ module Gitlab private def import_data_credentials - @import_data_credentials ||= project.import_data.credentials if project.import_data + @import_data_credentials ||= project.import_data.stringified_credentials if project.import_data end def user_map @@ -244,8 +242,8 @@ module Gitlab def build_attachment_url(rel_url) data = project.import_data.try(:data) - uri = data['fb_session']['uri'] - token = data['fb_session']['token'] + uri = import_data_credentials['fb_session']['uri'] + token = import_data_credentials['fb_session']['token'] "#{uri}/#{rel_url}&token=#{token}" end diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index 939cb96c101..0a87b406c56 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -25,9 +25,10 @@ module Gitlab ).execute import_data = project.import_data + import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } + # merge! with a bang doesn't work here import_data.credentials = import_data.credentials.merge('fb_session' => fb_session) - import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } import_data.save project -- cgit v1.2.1 From 6aeb753ba81c857c23d29f9d4c5e22aaeed737dc Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 1 Apr 2016 15:51:54 +0200 Subject: fix github import issues --- app/models/project.rb | 2 +- lib/gitlab/github_import/importer.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 52f70256be3..941e444a4f8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -418,7 +418,7 @@ class Project < ActiveRecord::Base def create_or_update_import_data(credentials) project_import_data = import_data || build_import_data project_import_data.credentials ||= {} - project_import_data.credentials.merge!(credentials) + project_import_data.credentials = project_import_data.credentials.merge(credentials) project_import_data.save end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index a5d3ab5fcf1..0b1ed510229 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -8,7 +8,7 @@ module Gitlab def initialize(project) @project = project if import_data_credentials - @client = Client.new(import_data_credentials['user']) + @client = Client.new(import_data_credentials[:user]) @formatter = Gitlab::ImportFormatter.new else raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" -- cgit v1.2.1 From 255cd31652f1afda5cd1c075526bbe3ee56a708e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 1 Apr 2016 17:27:07 +0200 Subject: fixes after more import testing --- lib/gitlab/bitbucket_import/client.rb | 2 +- lib/gitlab/gitlab_import/importer.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/bitbucket_import/client.rb b/lib/gitlab/bitbucket_import/client.rb index 1d1bd5e3216..acd0f298b3d 100644 --- a/lib/gitlab/bitbucket_import/client.rb +++ b/lib/gitlab/bitbucket_import/client.rb @@ -6,7 +6,7 @@ module Gitlab attr_reader :consumer, :api def self.from_project(project) - credentials = project.import_data.credentials if project.import_data + credentials = project.import_data.stringified_credentials if project.import_data if defined?(credentials) && credentials['bb_session'] token = credentials['bb_session']['bitbucket_access_token'] token_secret = credentials['bb_session']['bitbucket_access_token_secret'] diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index afc06d01ebd..19dc79462c0 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -5,7 +5,7 @@ module Gitlab def initialize(project) @project = project - credentials = import_data.credentials + credentials = import_data.stringified_credentials if credentials && credentials["password"] @client = Client.new(credentials["password"]) @formatter = Gitlab::ImportFormatter.new -- cgit v1.2.1 From 43ee65e173cbb4f846c118c531f635dc0f8602ac Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 1 Apr 2016 17:50:50 +0200 Subject: remove useless var --- lib/gitlab/fogbugz_import/importer.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index c88a44573a7..fffeb66ce26 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -241,7 +241,6 @@ module Gitlab end def build_attachment_url(rel_url) - data = project.import_data.try(:data) uri = import_data_credentials['fb_session']['uri'] token = import_data_credentials['fb_session']['token'] "#{uri}/#{rel_url}&token=#{token}" -- cgit v1.2.1 From ef85c510fa24a0deec6f680f9226dccd937a7bbe Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 4 Apr 2016 09:57:52 +0200 Subject: corrected a couple of based on MR review --- db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb | 4 ++-- lib/gitlab/fogbugz_import/importer.rb | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) 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 fb5d8591c09..60f86c74df0 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -16,8 +16,8 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration say("Projects and Github projects with a wrong URL. It also migrates Gitlab project credentials.") in_transaction { process_projects_with_wrong_url } - say("Migrating bitbucket credentials...")# TODO remove last param - in_transaction { process_project(import_type: 'bitbucket', unencrypted_data: ['repo', 'user_map']) } + say("Migrating bitbucket credentials...") + in_transaction { process_project(import_type: 'bitbucket') } say("Migrating fogbugz credentials...") in_transaction { process_project(import_type: 'fogbugz', unencrypted_data: ['repo', 'user_map']) } diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index fffeb66ce26..5c1c1c4865a 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -8,7 +8,7 @@ module Gitlab import_data = project.import_data.try(:data) repo_data = import_data['repo'] if import_data - if defined?(repo_data) + if repo_data @repo = FogbugzImport::Repository.new(repo_data) @known_labels = Set.new else @@ -18,7 +18,6 @@ module Gitlab def execute return true unless repo.valid? - Rails.logger.error import_data_credentials.inspect client = Gitlab::FogbugzImport::Client.new(token: import_data_credentials['fb_session']['token'], uri: import_data_credentials['fb_session']['uri']) @cases = client.cases(@repo.id.to_i) -- cgit v1.2.1 From 288d8e669883abe995fe27ad577189da42ef78fb Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 4 Apr 2016 18:03:55 +0200 Subject: fixes based on MR review --- ...152808_remove_wrong_import_url_from_projects.rb | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) 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 60f86c74df0..708dde8eca8 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -17,10 +17,10 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration in_transaction { process_projects_with_wrong_url } say("Migrating bitbucket credentials...") - in_transaction { process_project(import_type: 'bitbucket') } + in_transaction { process_project(import_type: 'bitbucket', credentials_keys: ['bb_session']) } say("Migrating fogbugz credentials...") - in_transaction { process_project(import_type: 'fogbugz', unencrypted_data: ['repo', 'user_map']) } + in_transaction { process_project(import_type: 'fogbugz', credentials_keys: ['fb_session']) } end @@ -33,27 +33,26 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end end - def process_project(import_type: , unencrypted_data: []) + def process_project(import_type: , credentials_keys: []) unencrypted_import_data(import_type: import_type).each do |data| - replace_data_credentials(data, unencrypted_data) + replace_data_credentials(data, credentials_keys) end end - def replace_data_credentials(data, unencrypted_data) + def replace_data_credentials(data, credentials_keys) data_hash = JSON.load(data['data']) if data['data'] - if defined?(data_hash) && !data_hash.blank? - unencrypted_data_hash = encrypted_data_hash(data_hash, unencrypted_data) - update_with_encrypted_data(data_hash, data['id'], unencrypted_data_hash) + unless data_hash.blank? + encrypted_data_hash = encrypt_data(data_hash, credentials_keys) + unencrypted_data = data_hash.empty? ? ' NULL ' : quote(data_hash.to_json) + update_with_encrypted_data(encrypted_data_hash, data['id'], unencrypted_data) end end - def encrypted_data_hash(data_hash, unencrypted_data) - return 'NULL' if unencrypted_data.empty? + def encrypt_data(data_hash, credentials_keys) new_data_hash = {} - unencrypted_data.each do |key| + credentials_keys.each do |key| new_data_hash[key] = data_hash.delete(key) if data_hash[key] end - quote(new_data_hash.to_json) end def in_transaction @@ -75,10 +74,10 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end end - def update_with_encrypted_data(data_hash, import_data_id, data_array = nil) + def update_with_encrypted_data(data_hash, import_data_id, unencrypted_data = ' NULL ') fake_import_data = FakeProjectImportData.new fake_import_data.credentials = data_hash - execute(update_import_data_sql(import_data_id, fake_import_data, data_array)) + execute(update_import_data_sql(import_data_id, fake_import_data, unencrypted_data)) end def update_import_url(import_url, project) -- cgit v1.2.1 From 172d37a2385424eb5f4c157fa691fd47b9f5a850 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 4 Apr 2016 19:45:53 +0200 Subject: fix wording --- .../20160302152808_remove_wrong_import_url_from_projects.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 708dde8eca8..0a7ba65acc4 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -12,14 +12,14 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def up 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.") + # 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 } - say("Migrating bitbucket credentials...") + say("Migrating Bitbucket credentials...") in_transaction { process_project(import_type: 'bitbucket', credentials_keys: ['bb_session']) } - say("Migrating fogbugz credentials...") + say("Migrating FogBugz credentials...") in_transaction { process_project(import_type: 'fogbugz', credentials_keys: ['fb_session']) } end @@ -109,7 +109,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration ).squish end - #Github projects with token, and any user:password@ based URL + #GitHub projects with token, and any user:password@ based URL #TODO: may need to add import_type != list def projects_with_wrong_import_url select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.id WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") -- cgit v1.2.1 From 4835e680a4624ab8de3316b367b8375bb5a270a0 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 4 Apr 2016 19:55:13 +0200 Subject: fix migration issue --- db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb | 1 + 1 file changed, 1 insertion(+) 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 0a7ba65acc4..8fef93233ef 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -53,6 +53,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration credentials_keys.each do |key| new_data_hash[key] = data_hash.delete(key) if data_hash[key] end + new_data_hash end def in_transaction -- cgit v1.2.1 From 5e51fce4dcd62997f372aed44badc844f98851e9 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 5 Apr 2016 15:41:15 +0200 Subject: some refactoring to symbolise keys across importers and left a TODO --- app/models/project_import_data.rb | 8 ++++++-- .../20160302152808_remove_wrong_import_url_from_projects.rb | 4 ++-- db/schema.rb | 5 ++--- lib/gitlab/bitbucket_import/client.rb | 8 ++++---- lib/gitlab/bitbucket_import/project_creator.rb | 2 +- lib/gitlab/fogbugz_import/importer.rb | 8 ++++---- lib/gitlab/fogbugz_import/project_creator.rb | 2 +- lib/gitlab/gitlab_import/importer.rb | 6 +++--- 8 files changed, 23 insertions(+), 20 deletions(-) diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index fa6055ff64d..225cbda15b1 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -18,7 +18,11 @@ class ProjectImportData < ActiveRecord::Base validates :project, presence: true - def stringified_credentials - JSON[credentials.to_json] + # TODO: This doesnt play well with attr_encrypted. Perhaps consider extending Marshall and specify a different Marshaller + before_validation :symbolize_credentials + + def symbolize_credentials + return if credentials.blank? + credentials.deep_symbolize_keys! 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 8fef93233ef..93e040fce28 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -51,9 +51,9 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def encrypt_data(data_hash, credentials_keys) new_data_hash = {} credentials_keys.each do |key| - new_data_hash[key] = data_hash.delete(key) if data_hash[key] + new_data_hash[key.to_sym] = data_hash.delete(key) if data_hash[key] end - new_data_hash + new_data_hash.deep_symbolize_keys end def in_transaction diff --git a/db/schema.rb b/db/schema.rb index df4c65a0625..5d87dbfe41f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160331133914) do +ActiveRecord::Schema.define(version: 20160331223143) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -44,7 +44,6 @@ ActiveRecord::Schema.define(version: 20160331133914) do t.datetime "updated_at" t.string "home_page_url" t.integer "default_branch_protection", default: 2 - t.boolean "twitter_sharing_enabled", default: true t.text "restricted_visibility_levels" t.boolean "version_check_enabled", default: true t.integer "max_attachment_size", default: 10, null: false @@ -417,9 +416,9 @@ ActiveRecord::Schema.define(version: 20160331133914) do t.string "state" t.integer "iid" t.integer "updated_by_id" - t.integer "moved_to_id" t.boolean "confidential", default: false t.datetime "deleted_at" + t.integer "moved_to_id" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree diff --git a/lib/gitlab/bitbucket_import/client.rb b/lib/gitlab/bitbucket_import/client.rb index acd0f298b3d..49f86ab5edf 100644 --- a/lib/gitlab/bitbucket_import/client.rb +++ b/lib/gitlab/bitbucket_import/client.rb @@ -6,10 +6,10 @@ module Gitlab attr_reader :consumer, :api def self.from_project(project) - credentials = project.import_data.stringified_credentials if project.import_data - if defined?(credentials) && credentials['bb_session'] - token = credentials['bb_session']['bitbucket_access_token'] - token_secret = credentials['bb_session']['bitbucket_access_token_secret'] + credentials = project.import_data if project.import_data + if credentials && credentials[:bb_session] + token = credentials[:bb_session][:bitbucket_access_token] + token_secret = credentials[:bb_session][:bitbucket_access_token_secret] new(token, token_secret) else raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index cc7f2017142..109010cb962 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -25,7 +25,7 @@ module Gitlab import_data = project.import_data # merge! with a bang doesn't work here - import_data.credentials = import_data.credentials.merge("bb_session" => session_data) + import_data.credentials = import_data.credentials.merge(bb_session: session_data) import_data.save project diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 5c1c1c4865a..249c5b48b1c 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -18,7 +18,7 @@ module Gitlab def execute return true unless repo.valid? - client = Gitlab::FogbugzImport::Client.new(token: import_data_credentials['fb_session']['token'], uri: import_data_credentials['fb_session']['uri']) + client = Gitlab::FogbugzImport::Client.new(token: import_data_credentials[:fb_session][:token], uri: import_data_credentials[:fb_session][:uri]) @cases = client.cases(@repo.id.to_i) @categories = client.categories @@ -31,7 +31,7 @@ module Gitlab private def import_data_credentials - @import_data_credentials ||= project.import_data.stringified_credentials if project.import_data + @import_data_credentials ||= project.import_data if project.import_data end def user_map @@ -240,8 +240,8 @@ module Gitlab end def build_attachment_url(rel_url) - uri = import_data_credentials['fb_session']['uri'] - token = import_data_credentials['fb_session']['token'] + uri = import_data_credentials[:fb_session][:uri] + token = import_data_credentials[:fb_session][:token] "#{uri}/#{rel_url}&token=#{token}" end diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index 0a87b406c56..e9fac8968e6 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -28,7 +28,7 @@ module Gitlab import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } # merge! with a bang doesn't work here - import_data.credentials = import_data.credentials.merge('fb_session' => fb_session) + import_data.credentials = import_data.credentials.merge(fb_session: fb_session) import_data.save project diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index 19dc79462c0..96717b42bae 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -5,9 +5,9 @@ module Gitlab def initialize(project) @project = project - credentials = import_data.stringified_credentials - if credentials && credentials["password"] - @client = Client.new(credentials["password"]) + credentials = import_data + if credentials && credentials[:password] + @client = Client.new(credentials[:password]) @formatter = Gitlab::ImportFormatter.new else raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" -- cgit v1.2.1 From b97654393e326095c7d95ccc1eb9f583a3b23da9 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 6 Apr 2016 10:36:30 +0200 Subject: fix some issues with credentials --- app/models/project_import_data.rb | 4 ++-- lib/gitlab/bitbucket_import/client.rb | 12 ++++++------ lib/gitlab/fogbugz_import/importer.rb | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 225cbda15b1..e984832685b 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -22,7 +22,7 @@ class ProjectImportData < ActiveRecord::Base before_validation :symbolize_credentials def symbolize_credentials - return if credentials.blank? - credentials.deep_symbolize_keys! + # bang doesn't work here + self.credentials = self.credentials.deep_symbolize_keys unless self.credentials.blank? end end diff --git a/lib/gitlab/bitbucket_import/client.rb b/lib/gitlab/bitbucket_import/client.rb index 49f86ab5edf..9bb507b5edd 100644 --- a/lib/gitlab/bitbucket_import/client.rb +++ b/lib/gitlab/bitbucket_import/client.rb @@ -6,10 +6,10 @@ module Gitlab attr_reader :consumer, :api def self.from_project(project) - credentials = project.import_data if project.import_data - if credentials && credentials[:bb_session] - token = credentials[:bb_session][:bitbucket_access_token] - token_secret = credentials[:bb_session][:bitbucket_access_token_secret] + import_data_credentials = project.import_data.credentials if project.import_data + if import_data_credentials && import_data_credentials[:bb_session] + token = import_data_credentials[:bb_session][:bitbucket_access_token] + token_secret = import_data_credentials[:bb_session][:bitbucket_access_token_secret] new(token, token_secret) else raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" @@ -65,7 +65,7 @@ module Gitlab def issues(project_identifier) all_issues = [] offset = 0 - per_page = 50 # Maximum number allowed by Bitbucket + per_page = 50 # Maximum number allowed by Bitbucket index = 0 begin @@ -131,7 +131,7 @@ module Gitlab end def config - Gitlab.config.omniauth.providers.find { |provider| provider.name == "bitbucket"} + Gitlab.config.omniauth.providers.find { |provider| provider.name == "bitbucket" } end def bitbucket_options diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 249c5b48b1c..42f9b6eab84 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -31,7 +31,7 @@ module Gitlab private def import_data_credentials - @import_data_credentials ||= project.import_data if project.import_data + @import_data_credentials ||= project.import_data.credentials if project.import_data end def user_map -- cgit v1.2.1 From 05be0c306ea8663461ee73023f162aeaf77a4325 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 6 Apr 2016 11:13:19 +0200 Subject: removed TODO [ci skip] --- app/models/project_import_data.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index e984832685b..a0994312003 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -18,7 +18,6 @@ class ProjectImportData < ActiveRecord::Base validates :project, presence: true - # TODO: This doesnt play well with attr_encrypted. Perhaps consider extending Marshall and specify a different Marshaller before_validation :symbolize_credentials def symbolize_credentials -- cgit v1.2.1 From 15044e7d857138b31199b796f02a81f0c29c643f Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 7 Apr 2016 15:00:20 +0200 Subject: refactored a few things based on MR feedback --- app/models/project.rb | 13 +++++++++++++ db/schema.rb | 8 ++++---- lib/gitlab/bitbucket_import/project_creator.rb | 5 +---- lib/gitlab/fogbugz_import/importer.rb | 10 +++++----- lib/gitlab/fogbugz_import/project_creator.rb | 7 +------ lib/gitlab/google_code_import/project_creator.rb | 4 +--- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 7b1188420ef..17b971b9d30 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -424,6 +424,19 @@ class Project < ActiveRecord::Base project_import_data.save end + def create_or_update_import_data(credentials) + project_import_data = import_data || build_import_data + project_import_data.credentials ||= {} + project_import_data.credentials = project_import_data.credentials.merge(credentials) + project_import_data.save + end + + def update_import_data(data: nil, credentials: nil) + import_data.data = data if data + import_data.credentials = import_data.credentials.merge(credentials) if credentials + import_data.save + end + def import? external_import? || forked? end diff --git a/db/schema.rb b/db/schema.rb index 5d87dbfe41f..53d12aa35dd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160331223143) do +ActiveRecord::Schema.define(version: 20160320204112) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -44,6 +44,7 @@ ActiveRecord::Schema.define(version: 20160331223143) do t.datetime "updated_at" t.string "home_page_url" t.integer "default_branch_protection", default: 2 + t.boolean "twitter_sharing_enabled", default: true t.text "restricted_visibility_levels" t.boolean "version_check_enabled", default: true t.integer "max_attachment_size", default: 10, null: false @@ -416,9 +417,9 @@ ActiveRecord::Schema.define(version: 20160331223143) do t.string "state" t.integer "iid" t.integer "updated_by_id" - t.boolean "confidential", default: false - t.datetime "deleted_at" t.integer "moved_to_id" + t.boolean "confidential", default: false + t.datetime "deleted_at" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -747,7 +748,6 @@ ActiveRecord::Schema.define(version: 20160331223143) do add_index "projects", ["namespace_id"], name: "index_projects_on_namespace_id", using: :btree add_index "projects", ["path"], name: "index_projects_on_path", using: :btree add_index "projects", ["path"], name: "index_projects_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"} - add_index "projects", ["pending_delete"], name: "index_projects_on_pending_delete", using: :btree add_index "projects", ["runners_token"], name: "index_projects_on_runners_token", using: :btree add_index "projects", ["star_count"], name: "index_projects_on_star_count", using: :btree add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 109010cb962..65b62b2b816 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -23,10 +23,7 @@ module Gitlab import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git", ).execute - import_data = project.import_data - # merge! with a bang doesn't work here - import_data.credentials = import_data.credentials.merge(bb_session: session_data) - import_data.save + project.update_import_data(credentials: { bb_session: session_data }) project end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 42f9b6eab84..501d5a95547 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -18,7 +18,7 @@ module Gitlab def execute return true unless repo.valid? - client = Gitlab::FogbugzImport::Client.new(token: import_data_credentials[:fb_session][:token], uri: import_data_credentials[:fb_session][:uri]) + client = Gitlab::FogbugzImport::Client.new(token: fb_session[:token], uri: fb_session[:uri]) @cases = client.cases(@repo.id.to_i) @categories = client.categories @@ -30,8 +30,8 @@ module Gitlab private - def import_data_credentials - @import_data_credentials ||= project.import_data.credentials if project.import_data + def fb_session + @import_data_credentials ||= project.import_data.credentials[:fb_session] if project.import_data && project.import_data.credentials end def user_map @@ -240,8 +240,8 @@ module Gitlab end def build_attachment_url(rel_url) - uri = import_data_credentials[:fb_session][:uri] - token = import_data_credentials[:fb_session][:token] + uri = fb_session[:uri] + token = fb_session[:token] "#{uri}/#{rel_url}&token=#{token}" end diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index e9fac8968e6..c000b300468 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -24,12 +24,7 @@ module Gitlab import_url: Project::UNKNOWN_IMPORT_URL ).execute - import_data = project.import_data - import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } - - # merge! with a bang doesn't work here - import_data.credentials = import_data.credentials.merge(fb_session: fb_session) - import_data.save + project.update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }, credentials: { fb_session: fb_session }) project end diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb index 49d6013af28..d2e20afbb1e 100644 --- a/lib/gitlab/google_code_import/project_creator.rb +++ b/lib/gitlab/google_code_import/project_creator.rb @@ -24,9 +24,7 @@ module Gitlab import_url: repo.import_url ).execute - import_data = project.import_data - import_data.data = { 'repo' => repo.raw_data, 'user_map' => user_map } - import_data.save + project.update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }) project end -- cgit v1.2.1 From a1a1d1f7de71f46787f12f1efa23346a2a6b1c29 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 7 Apr 2016 15:08:38 +0200 Subject: refactored create_or_update_import_data --- app/models/project.rb | 23 +++++++---------------- lib/gitlab/bitbucket_import/project_creator.rb | 2 +- lib/gitlab/fogbugz_import/project_creator.rb | 2 +- lib/gitlab/google_code_import/project_creator.rb | 2 +- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 17b971b9d30..273b04c6323 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -404,7 +404,7 @@ class Project < ActiveRecord::Base def import_url=(value) import_url = Gitlab::ImportUrl.new(value) - create_or_update_import_data(import_url.credentials) + create_or_update_import_data(credentials: import_url.credentials) super(import_url.sanitized_url) end @@ -417,26 +417,17 @@ class Project < ActiveRecord::Base end end - def create_or_update_import_data(credentials) + def create_or_update_import_data(data: nil, credentials: nil) project_import_data = import_data || build_import_data - project_import_data.credentials ||= {} - project_import_data.credentials = project_import_data.credentials.merge(credentials) - project_import_data.save - end + project_import_data.data = data if data + if credentials + project_import_data.credentials ||= {} + project_import_data.credentials = project_import_data.credentials.merge(credentials) + end - def create_or_update_import_data(credentials) - project_import_data = import_data || build_import_data - project_import_data.credentials ||= {} - project_import_data.credentials = project_import_data.credentials.merge(credentials) project_import_data.save end - def update_import_data(data: nil, credentials: nil) - import_data.data = data if data - import_data.credentials = import_data.credentials.merge(credentials) if credentials - import_data.save - end - def import? external_import? || forked? end diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 65b62b2b816..941f818b847 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -23,7 +23,7 @@ module Gitlab import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git", ).execute - project.update_import_data(credentials: { bb_session: session_data }) + project.create_or_update_import_data(credentials: { bb_session: session_data }) project end diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index c000b300468..3840765db87 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -24,7 +24,7 @@ module Gitlab import_url: Project::UNKNOWN_IMPORT_URL ).execute - project.update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }, credentials: { fb_session: fb_session }) + project.create_or_update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }, credentials: { fb_session: fb_session }) project end diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb index d2e20afbb1e..0abb7a64c17 100644 --- a/lib/gitlab/google_code_import/project_creator.rb +++ b/lib/gitlab/google_code_import/project_creator.rb @@ -24,7 +24,7 @@ module Gitlab import_url: repo.import_url ).execute - project.update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }) + project.create_or_update_import_data(data: { 'repo' => repo.raw_data, 'user_map' => user_map }) project end -- cgit v1.2.1 From 0ceee903f14d1824aa56452b83ae4a8e090cbc14 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 7 Apr 2016 15:49:46 +0200 Subject: fix schema file [ci skip] --- db/schema.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 53d12aa35dd..df4c65a0625 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160320204112) do +ActiveRecord::Schema.define(version: 20160331133914) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -418,7 +418,7 @@ ActiveRecord::Schema.define(version: 20160320204112) do t.integer "iid" t.integer "updated_by_id" t.integer "moved_to_id" - t.boolean "confidential", default: false + t.boolean "confidential", default: false t.datetime "deleted_at" end @@ -748,6 +748,7 @@ ActiveRecord::Schema.define(version: 20160320204112) do add_index "projects", ["namespace_id"], name: "index_projects_on_namespace_id", using: :btree add_index "projects", ["path"], name: "index_projects_on_path", using: :btree add_index "projects", ["path"], name: "index_projects_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"} + add_index "projects", ["pending_delete"], name: "index_projects_on_pending_delete", using: :btree add_index "projects", ["runners_token"], name: "index_projects_on_runners_token", using: :btree add_index "projects", ["star_count"], name: "index_projects_on_star_count", using: :btree add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree -- cgit v1.2.1 From 350a9aa984100a4d91454200fb4ac308108e5cef Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 7 Apr 2016 16:53:15 +0200 Subject: fix create_or_update_import_data --- app/models/project.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 273b04c6323..6304699386d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -419,7 +419,10 @@ class Project < ActiveRecord::Base def create_or_update_import_data(data: nil, credentials: nil) project_import_data = import_data || build_import_data - project_import_data.data = data if data + if data + project_import_data.data ||= {} + project_import_data.data = project_import_data.data.merge(data) + end if credentials project_import_data.credentials ||= {} project_import_data.credentials = project_import_data.credentials.merge(credentials) -- cgit v1.2.1 From be834c4de93a7716035f5373210ea3922c26da72 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 11 Apr 2016 11:13:51 +0200 Subject: changed a few things based on feedback --- app/models/project_import_data.rb | 8 ++++++-- ...add_import_credentials_to_project_import_data.rb | 4 ++-- ...2152808_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 -- cgit v1.2.1 From fb63173df2bf81c155ed311d2c8fc5889a5faf1d Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 11 Apr 2016 11:28:35 +0200 Subject: typo --- db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dd2842b835e..b97b79f920d 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -110,7 +110,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration #GitHub projects with token, and any user:password@ based URL #TODO: may need to add import_type != list def projects_with_wrong_import_url - select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.id WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") + select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") end # All imports with data for import_type -- cgit v1.2.1 From efff7e9e54e1d1ebbb36dc8dca35b894aeab2630 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 14 Apr 2016 12:03:45 +0200 Subject: updated migration based on testing findings --- ...302152808_remove_wrong_import_url_from_projects.rb | 19 +++++++++++++------ lib/gitlab/import_url.rb | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) 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 b97b79f920d..8a351cf27a3 100644 --- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb +++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb @@ -23,10 +23,14 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration def process_projects_with_wrong_url projects_with_wrong_import_url.each do |project| - import_url = Gitlab::ImportUrl.new(project["import_url"]) + begin + import_url = Gitlab::ImportUrl.new(project["import_url"]) - update_import_url(import_url, project) - update_import_data(import_url, project) + update_import_url(import_url, project) + update_import_data(import_url, project) + rescue URI::InvalidURIError + nullify_import_url(project) + end end end @@ -82,6 +86,10 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration execute("UPDATE projects SET import_url = #{quote(import_url.sanitized_url)} WHERE id = #{project['id']}") end + def nullify_import_url(project) + execute("UPDATE projects SET import_url = NULL WHERE id = #{project['id']}") + end + def insert_import_data_sql(project_id, fake_import_data) %( INSERT INTO project_import_data @@ -108,14 +116,13 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration end #GitHub projects with token, and any user:password@ based URL - #TODO: may need to add import_type != list def projects_with_wrong_import_url - select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'") + select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.project_id WHERE p.import_url <> '' AND p.import_url LIKE '%//%@%'") end # All imports with data for import_type def unencrypted_import_data(import_type: ) - select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = '#{import_type}' ") + select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url <> '' AND p.import_type = '#{import_type}' ") end def quote(value) diff --git a/lib/gitlab/import_url.rb b/lib/gitlab/import_url.rb index 3cfbc17b89b..d23b013c1f5 100644 --- a/lib/gitlab/import_url.rb +++ b/lib/gitlab/import_url.rb @@ -1,7 +1,7 @@ module Gitlab class ImportUrl def initialize(url, credentials: nil) - @url = URI.parse(url) + @url = URI.parse(URI.encode(url)) @credentials = credentials end -- cgit v1.2.1 From 46d1cf43a43a4d7a25f25be97d1fee79cefdc773 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 14 Apr 2016 18:05:53 +0200 Subject: update changelog [ci skip] --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 1d1e541e65f..cfe47a4e56f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -29,6 +29,7 @@ v 8.7.0 (unreleased) - Fix admin/projects when using visibility levels on search (PotHix) - Build status notifications - API: Expose user location (Robert Schilling) + - Add encrypted credentials for imported projects and migrate old ones v 8.6.5 (unreleased) - Only update repository language if it is not set to improve performance -- cgit v1.2.1 From ca725aaf4ca7cb300c50d539fe27efc45a32dfd6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 15 Apr 2016 08:52:26 +0200 Subject: resolve merge conflict --- CHANGELOG | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d0638c880cf..6de5a9b9389 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -57,11 +57,6 @@ v 8.7.0 (unreleased) - Fix admin/projects when using visibility levels on search (PotHix) - Build status notifications - API: Expose user location (Robert Schilling) - - Add encrypted credentials for imported projects and migrate old ones - -v 8.6.5 (unreleased) - - Only update repository language if it is not set to improve performance - - Check permissions when user attempts to import members from another project - API: Do not leak group existence via return code (Robert Schilling) - ClosingIssueExtractor regex now also works with colons. e.g. "Fixes: #1234" !3591 - Update number of Todos in the sidebar when it's marked as "Done". !3600 @@ -73,6 +68,7 @@ v 8.6.5 (unreleased) - Diffs load at the correct point when linking from from number - Selected diff rows highlight - Fix emoji catgories in the emoji picker + - Add encrypted credentials for imported projects and migrate old ones v 8.6.6 - Fix error on language detection when repository has no HEAD (e.g., master branch). !3654 (Jeroen Bobbeldijk) -- cgit v1.2.1