Merge branch 'issue_447' into 'master'

Mask credentials in URLs instead of remove them

REF: #447 

REF: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/382#note_11713978

See merge request !385

app/models/project.rb

@@ -519,12 +519,7 @@ class Project < ActiveRecord::Base
   end
 
   def safe_import_url
-    result = URI.parse(self.import_url)
-    result.password = '*****' unless result.password.nil?
-    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
+    Gitlab::UrlSanitizer.new(import_url).masked_url
   end
 
   def mirror_updated?

lib/gitlab/url_sanitizer.rb

@@ -3,7 +3,7 @@ module Gitlab
     def self.sanitize(content)
       regexp = URI::Parser.new.make_regexp(['http', 'https', 'ssh', 'git'])
 
-      content.gsub(regexp) { |url| new(url).sanitized_url }
+      content.gsub(regexp) { |url| new(url).masked_url }
     end
 
     def initialize(url, credentials: nil)
@@ -15,6 +15,13 @@ module Gitlab
       @sanitized_url ||= safe_url.to_s
     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
       @credentials ||= { user: @url.user, password: @url.password }
     end

spec/lib/gitlab/url_sanitizer_spec.rb

@@ -31,16 +31,16 @@ describe Gitlab::UrlSanitizer, lib: true do
       })
     end
 
-    it 'remove credentials from HTTP URLs' do
-      expect(filtered_content).to include("http://test.com/root/repoC.git/")
+    it 'mask the credentials from HTTP URLs' do
+      expect(filtered_content).to include("http://*****:*****@test.com/root/repoC.git/")
     end
 
-    it 'remove credentials from HTTPS URLs' do
-      expect(filtered_content).to include("https://test.com/root/repoA.git/")
+    it 'mask the credentials from HTTPS URLs' do
+      expect(filtered_content).to include("https://*****:*****@test.com/root/repoA.git/")
     end
 
-    it 'remove credentials from SSH URLs' do
-      expect(filtered_content).to include("ssh://host.test/path/to/repo.git")
+    it 'mask credentials from SSH URLs' do
+      expect(filtered_content).to include("ssh://*****@host.test/path/to/repo.git")
     end
 
     it 'does not modify Git URLs' do