From 383cc8404741f65bd52fbe80eec6c2dae2578fce Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 21 Mar 2016 13:15:51 +0100 Subject: [PATCH] some refactoring based on feedback --- app/models/project.rb | 12 +++++ ...8_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 -- GitLab