Merge branch 'fix/project-import_url' into 'master'

Fix for import_url fields on projects containing third-party credentials

Fixes https://dev.gitlab.org/gitlab/gitlabhq/issues/2658 

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/13955

See merge request !3066

CHANGELOG

@@ -73,6 +73,7 @@ v 8.7.0 (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
   - Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413

app/models/project.rb

@@ -409,6 +409,35 @@ class Project < ActiveRecord::Base
     self.import_data.destroy if self.import_data
   end
 
+  def import_url=(value)
+    import_url = Gitlab::ImportUrl.new(value)
+    create_or_update_import_data(credentials: import_url.credentials)
+    super(import_url.sanitized_url)
+  end
+
+  def import_url
+    if import_data && super
+      import_url = Gitlab::ImportUrl.new(super, credentials: import_data.credentials)
+      import_url.full_url
+    else
+      super
+    end
+  end
+
+  def create_or_update_import_data(data: nil, credentials: nil)
+    project_import_data = import_data || build_import_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)
+    end
+
+    project_import_data.save
+  end
+
   def import?
     external_import? || forked?
   end

app/models/project_import_data.rb

@@ -12,8 +12,20 @@ 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
+
   serialize :data, JSON
 
   validates :project, presence: true
+
+  before_validation :symbolize_credentials
+
+  def symbolize_credentials
+    # 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

db/migrate/20160302151724_add_import_credentials_to_project_import_data.rb

+class AddImportCredentialsToProjectImportData < ActiveRecord::Migration
+  def change
+    add_column :project_import_data, :encrypted_credentials, :text
+    add_column :project_import_data, :encrypted_credentials_iv, :string
+    add_column :project_import_data, :encrypted_credentials_salt, :string
+  end
+end

db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb

+# 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 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
+  end
+
+  def up
+    say("Encrypting and migrating project import credentials...")
+
+    # This should cover GitHub, GitLab, Bitbucket user:password, token@domain, and other similar URLs.
+    in_transaction(message: "Projects including GitHub and GitLab projects with an unsecured URL.") { process_projects_with_wrong_url }
+
+    in_transaction(message: "Migrating Bitbucket credentials...") { process_project(import_type: 'bitbucket', credentials_keys: ['bb_session']) }
+
+    in_transaction(message: "Migrating FogBugz credentials...") { process_project(import_type: 'fogbugz', credentials_keys: ['fb_session']) }
+
+  end
+
+  def process_projects_with_wrong_url
+    projects_with_wrong_import_url.each do |project|
+      begin
+        import_url = Gitlab::ImportUrl.new(project["import_url"])
+
+        update_import_url(import_url, project)
+        update_import_data(import_url, project)
+      rescue URI::InvalidURIError
+        nullify_import_url(project)
+      end
+    end
+  end
+
+  def process_project(import_type:, credentials_keys: [])
+    unencrypted_import_data(import_type: import_type).each do |data|
+      replace_data_credentials(data, credentials_keys)
+    end
+  end
+
+  def replace_data_credentials(data, credentials_keys)
+    data_hash = JSON.load(data['data']) if data['data']
+    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 encrypt_data(data_hash, credentials_keys)
+    new_data_hash = {}
+    credentials_keys.each do |key|
+      new_data_hash[key.to_sym] = data_hash.delete(key) if data_hash[key]
+    end
+    new_data_hash.deep_symbolize_keys
+  end
+
+  def in_transaction(message:)
+    say_with_time(message) do
+      ActiveRecord::Base.transaction do
+        yield
+      end
+    end
+  end
+
+  def update_import_data(import_url, project)
+    fake_import_data = ProjectImportDataFake.new
+    fake_import_data.credentials = import_url.credentials
+    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, unencrypted_data = ' NULL ')
+    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
+
+  def update_import_url(import_url, project)
+    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
+                  (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}'
+    ).squish
+  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, 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 <> '' AND p.import_type = '#{import_type}' ")
+  end
+
+  def quote(value)
+    ActiveRecord::Base.connection.quote(value)
+  end
+end

db/schema.rb

@@ -704,6 +704,9 @@ ActiveRecord::Schema.define(version: 20160412140240) 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"
+    t.text    "encrypted_credentials_salt"
   end
 
   create_table "projects", force: :cascade do |t|

lib/gitlab/bitbucket_import/client.rb

@@ -5,6 +5,17 @@ module Gitlab
 
       attr_reader :consumer, :api
 
+      def self.from_project(project)
+        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}"
+        end
+      end
+
       def initialize(access_token = nil, access_token_secret = nil)
         @consumer = ::OAuth::Consumer.new(
           config.app_id,
@@ -54,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
@@ -120,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

lib/gitlab/bitbucket_import/importer.rb

@@ -5,10 +5,7 @@ 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"])
+        @client = Client.from_project(@project)
         @formatter = Gitlab::ImportFormatter.new
       end
 

lib/gitlab/bitbucket_import/key_deleter.rb

@@ -6,10 +6,7 @@ module Gitlab
       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"])
+        @client = Client.from_project(@project)
       end
 
       def execute

lib/gitlab/bitbucket_import/project_creator.rb

@@ -23,7 +23,8 @@ 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_or_update_import_data(credentials: { bb_session: session_data })
+
         project
       end
     end

lib/gitlab/fogbugz_import/importer.rb

@@ -8,17 +8,17 @@ 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 repo_data
+          @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
         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'])
+        client = Gitlab::FogbugzImport::Client.new(token: fb_session[:token], uri: fb_session[:uri])
 
         @cases = client.cases(@repo.id.to_i)
         @categories = client.categories
@@ -30,6 +30,10 @@ module Gitlab
 
       private
 
+      def fb_session
+        @import_data_credentials ||= project.import_data.credentials[:fb_session] if project.import_data && project.import_data.credentials
+      end
+
       def user_map
         @user_map ||= begin
           user_map = Hash.new
@@ -236,9 +240,8 @@ module Gitlab
       end
 
       def build_attachment_url(rel_url)
-        data = project.import_data.try(:data)
-        uri = data['fb_session']['uri']
-        token = data['fb_session']['token']
+        uri = fb_session[:uri]
+        token = fb_session[:token]
         "#{uri}/#{rel_url}&token=#{token}"
       end
 

lib/gitlab/fogbugz_import/project_creator.rb

@@ -24,13 +24,7 @@ module Gitlab
           import_url: Project::UNKNOWN_IMPORT_URL
         ).execute
 
-        project.create_import_data(
-          data: {
-            'repo' => repo.raw_data,
-            'user_map' => user_map,
-            '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

lib/gitlab/github_import/importer.rb

@@ -7,10 +7,12 @@ module Gitlab
 
       def initialize(project)
         @project = project
-        import_data = project.import_data.try(:data)
-        github_session = import_data["github_session"] if import_data
-        @client = Client.new(github_session["github_access_token"])
-        @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
@@ -19,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,

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,
@@ -23,9 +23,6 @@ module Gitlab
           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
-
-        project.create_import_data(data: { "github_session" => session_data } )
-        project
       end
     end
   end

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
+        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

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

lib/gitlab/google_code_import/project_creator.rb

@@ -24,12 +24,7 @@ module Gitlab
           import_url: repo.import_url
         ).execute
 
-        project.create_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

lib/gitlab/import_url.rb

+module Gitlab
+  class ImportUrl
+    def initialize(url, credentials: nil)
+      @url = URI.parse(URI.encode(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
+      return @url unless valid_credentials?
+      @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
+
+    def valid_credentials?
+      credentials && credentials.is_a?(Hash) && credentials.any?
+    end
+  end
+end

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) }

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,6 +27,8 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do
     project = project_creator.execute
 
     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

spec/lib/gitlab/github_import/wiki_formatter_spec.rb

@@ -2,11 +2,12 @@ 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')
+    create(:project,
+           namespace: create(:namespace, path: 'gitlabhq'),
+           import_url: 'https://xxx@github.com/gitlabhq/sample.gitlabhq.git')
   end
 
-  subject(:wiki) { described_class.new(project)}
+  subject(:wiki) { described_class.new(project) }
 
   describe '#path_with_namespace' do
     it 'appends .wiki to project path' do

spec/lib/gitlab/import_url_spec.rb

+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