Commit 73556332 authored by James Lopez's avatar James Lopez

refactored a bunch of stuff based on MR feedback

parent 7085850c
...@@ -12,7 +12,7 @@ require 'file_size_validator' ...@@ -12,7 +12,7 @@ require 'file_size_validator'
class ProjectImportData < ActiveRecord::Base class ProjectImportData < ActiveRecord::Base
belongs_to :project 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 serialize :data, JSON
......
...@@ -2,43 +2,49 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration ...@@ -2,43 +2,49 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
class ImportUrlSanitizer class ImportUrlSanitizer
def initialize(url) def initialize(url)
@url = url @url = URI.parse(url)
end end
def sanitized_url def sanitized_url
@sanitized_url ||= @url[regex_extractor, 1] + @url[regex_extractor, 3] @sanitized_url ||= safe_url
end end
def credentials def credentials
@credentials ||= @url[regex_extractor, 2] @credentials ||= { user: @url.user, password: @url.password }
end end
private private
# Regex matches 1 <first part of URL>, 2 <token or to be encrypted stuff>, def safe_url
# 3 <last part of URL> safe_url = @url.dup
def regex_extractor safe_url.password = nil
/(.*\/\/)(.*)(\@.*)/ safe_url.user = nil
safe_url
end end
end
class FakeProjectImportData
extend AttrEncrypted
attr_accessor :credentials
attr_encrypted :credentials, key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true
end end
def up def up
projects_with_wrong_import_url.each do |project_id| projects_with_wrong_import_url.each do |project|
project = Project.find(project_id["id"]) sanitizer = ImportUrlSanitizer.new(project["import_url"])
sanitizer = ImportUrlSanitizer.new(project.import_url)
ActiveRecord::Base.transaction do ActiveRecord::Base.transaction do
project.update_columns(import_url: sanitizer.sanitized_url) execute("UPDATE projects SET import_url = '#{sanitizer.sanitized_url}' WHERE id = #{project['id']}")
if project.import_data fake_import_data = FakeProjectImportData.new
project.import_data.credentials = sanitizer.credentials fake_import_data.credentials = sanitizer.credentials
project.save! execute("UPDATE project_import_data SET encrypted_credentials = '#{fake_import_data.encrypted_credentials}' WHERE project_id = #{project['id']}")
end
end end
end end
end end
def projects_with_wrong_import_url def projects_with_wrong_import_url
# TODO Check live with #operations for possible false positives. Also, consider regex? But may have issues MySQL/PSQL # 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
end end
...@@ -7,8 +7,8 @@ module Gitlab ...@@ -7,8 +7,8 @@ module Gitlab
def initialize(project) def initialize(project)
@project = project @project = project
github_session = project.import_data.credentials if import_data credentials = project.import_data.credentials if import_data
@client = Client.new(github_session["github_access_token"]) @client = Client.new(credentials["github_access_token"])
@formatter = Gitlab::ImportFormatter.new @formatter = Gitlab::ImportFormatter.new
end end
......
...@@ -32,8 +32,7 @@ module Gitlab ...@@ -32,8 +32,7 @@ module Gitlab
def create_import_data(project) def create_import_data(project)
project.create_import_data( project.create_import_data(
credentials: { github_access_token: session_data.delete(:github_access_token) }, credentials: { github_access_token: session_data.delete(:github_access_token) })
data: { github_session: session_data })
end end
end end
end end
......
...@@ -2,16 +2,12 @@ module Gitlab ...@@ -2,16 +2,12 @@ module Gitlab
# Exposes an import URL that includes the credentials unencrypted. # Exposes an import URL that includes the credentials unencrypted.
# Extracted to its own class to prevent unintended use. # Extracted to its own class to prevent unintended use.
module ImportUrlExposer module ImportUrlExposer
extend self
def expose(import_url:, credentials: ) def self.expose(import_url:, credentials: )
import_url.sub("//", "//#{parsed_credentials(credentials)}@") uri = URI.parse(import_url)
end uri.user = credentials[:user]
uri.password = credentials[:password]
private uri
def parsed_credentials(credentials)
credentials.values.join(":")
end end
end end
end end
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment