Read about our upcoming Code of Conduct on this issue

Commit 0c47b68d authored by Rubén Dávila's avatar Rubén Dávila
Browse files

Mask credentials from URL when import of project has failed.

parent 3962fc1e01ac
...@@ -376,14 +376,14 @@ def clear_import_data ...@@ -376,14 +376,14 @@ def clear_import_data
end end
def import_url=(value) def import_url=(value)
import_url = Gitlab::ImportUrl.new(value) import_url = Gitlab::UrlSanitizer.new(value)
create_or_update_import_data(credentials: import_url.credentials) create_or_update_import_data(credentials: import_url.credentials)
super(import_url.sanitized_url) super(import_url.sanitized_url)
end end
def import_url def import_url
if import_data && super if import_data && super
import_url = Gitlab::ImportUrl.new(super, credentials: import_data.credentials) import_url = Gitlab::UrlSanitizer.new(super, credentials: import_data.credentials)
import_url.full_url import_url.full_url
else else
super super
......
...@@ -13,7 +13,7 @@ def perform(project_id) ...@@ -13,7 +13,7 @@ def perform(project_id)
result = Projects::ImportService.new(project, current_user).execute result = Projects::ImportService.new(project, current_user).execute
if result[:status] == :error if result[:status] == :error
project.update(import_error: result[:message]) project.update(import_error: Gitlab::UrlSanitizer.sanitize(result[:message]))
project.import_fail project.import_fail
return return
end end
......
...@@ -24,7 +24,7 @@ def up ...@@ -24,7 +24,7 @@ def up
def process_projects_with_wrong_url def process_projects_with_wrong_url
projects_with_wrong_import_url.each do |project| projects_with_wrong_import_url.each do |project|
begin begin
import_url = Gitlab::ImportUrl.new(project["import_url"]) import_url = Gitlab::UrlSanitizer.new(project["import_url"])
update_import_url(import_url, project) update_import_url(import_url, project)
update_import_data(import_url, project) update_import_data(import_url, project)
......
module Gitlab module Gitlab
class ImportUrl class UrlSanitizer
def self.sanitize(content)
regexp = URI::Parser.new.make_regexp(['http', 'https', 'ssh', 'git'])
content.gsub(regexp) { |url| new(url).masked_url }
end
def initialize(url, credentials: nil) def initialize(url, credentials: nil)
@url = URI.parse(URI.encode(url)) @url = Addressable::URI.parse(URI.encode(url))
@credentials = credentials @credentials = credentials
end end
...@@ -9,6 +15,13 @@ def sanitized_url ...@@ -9,6 +15,13 @@ def sanitized_url
@sanitized_url ||= safe_url.to_s @sanitized_url ||= safe_url.to_s
end end
def masked_url
url = @url.dup
url.password = "*****" unless url.password.nil?
url.user = "*****" unless url.user.nil?
url.to_s
end
def credentials def credentials
@credentials ||= { user: @url.user, password: @url.password } @credentials ||= { user: @url.user, password: @url.password }
end end
......
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
require 'spec_helper'
describe Gitlab::UrlSanitizer, lib: true do
let(:credentials) { { user: 'blah', password: 'password' } }
let(:url_sanitizer) do
described_class.new("https://github.com/me/project.git", credentials: credentials)
end
describe '.sanitize' do
def sanitize_url(url)
# We want to try with multi-line content because is how error messages are formatted
described_class.sanitize(%Q{
remote: Not Found
fatal: repository '#{url}' not found
})
end
it 'mask the credentials from HTTP URLs' do
filtered_content = sanitize_url('http://user:pass@test.com/root/repoC.git/')
expect(filtered_content).to include("http://*****:*****@test.com/root/repoC.git/")
end
it 'mask the credentials from HTTPS URLs' do
filtered_content = sanitize_url('https://user:pass@test.com/root/repoA.git/')
expect(filtered_content).to include("https://*****:*****@test.com/root/repoA.git/")
end
it 'mask credentials from SSH URLs' do
filtered_content = sanitize_url('ssh://user@host.test/path/to/repo.git')
expect(filtered_content).to include("ssh://*****@host.test/path/to/repo.git")
end
it 'does not modify Git URLs' do
# git protocol does not support authentication
filtered_content = sanitize_url('git://host.test/path/to/repo.git')
expect(filtered_content).to include("git://host.test/path/to/repo.git")
end
it 'does not modify scp-like URLs' do
filtered_content = sanitize_url('user@server:project.git')
expect(filtered_content).to include("user@server:project.git")
end
end
describe '#sanitized_url' do
it { expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") }
end
describe '#credentials' do
it { expect(url_sanitizer.credentials).to eq(credentials) }
end
describe '#full_url' do
it { expect(url_sanitizer.full_url).to eq("https://blah:password@github.com/me/project.git") }
it 'supports scp-like URLs' do
sanitizer = described_class.new('user@server:project.git')
expect(sanitizer.full_url).to eq('user@server:project.git')
end
end
end
...@@ -6,14 +6,28 @@ ...@@ -6,14 +6,28 @@
subject { described_class.new } subject { described_class.new }
describe '#perform' do describe '#perform' do
it 'imports a project' do context 'when the import was successful' do
expect_any_instance_of(Projects::ImportService).to receive(:execute). it 'imports a project' do
and_return({ status: :ok }) expect_any_instance_of(Projects::ImportService).to receive(:execute).
and_return({ status: :ok })
expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) expect_any_instance_of(Repository).to receive(:expire_emptiness_caches)
expect_any_instance_of(Project).to receive(:import_finish) expect_any_instance_of(Project).to receive(:import_finish)
subject.perform(project.id) subject.perform(project.id)
end
end
context 'when the import has failed' do
it 'hide the credentials that were used in the import URL' do
error = %Q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found }
expect_any_instance_of(Projects::ImportService).to receive(:execute).
and_return({ status: :error, message: error })
subject.perform(project.id)
expect(project.reload.import_error).to include("https://*****:*****@test.com/root/repoC.git/")
end
end end
end end
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