kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Remove blank passwords from sanitized URLs

Browse source
This commit is contained in:
Nick Thomas 2017-09-06 15:20:25 +01:00
parent 759f34bd0a
commit c8bdb20228
3 changed files with 154 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
changelogs/unreleased/url-sanitizer-fixes.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Fix problems sanitizing URLs with empty passwords
merge_request: 14083
author:
type: fixed

13
lib/gitlab/url_sanitizer.rb
View file

@ -19,7 +19,12 @@ module Gitlab
end
def initialize(url, credentials: nil)
@url = Addressable::URI.parse(url.strip)
@url = Addressable::URI.parse(url.to_s.strip)
%i[user password].each do |symbol|
credentials[symbol] = credentials[symbol].presence if credentials&.key?(symbol)
end
@credentials = credentials
end
@ -47,8 +52,10 @@ module Gitlab
def generate_full_url
return @url unless valid_credentials?
@full_url = @url.dup
@full_url.user = credentials[:user].presence
@full_url.password = credentials[:password].presence
@full_url.password = credentials[:password]
@full_url.user = credentials[:user]
@full_url
end

200
spec/lib/gitlab/url_sanitizer_spec.rb
View file

@ -1,11 +1,7 @@
require 'spec_helper'
describe Gitlab::UrlSanitizer do
let(:credentials) { { user: 'blah', password: 'password' } }
let(:url_sanitizer) do
described_class.new("https://github.com/me/project.git", credentials: credentials)
end
let(:user) { double(:user, username: 'john.doe') }
using RSpec::Parameterized::TableSyntax
describe '.sanitize' do
def sanitize_url(url)
@ -16,84 +12,166 @@ describe Gitlab::UrlSanitizer do
})
end
it 'mask the credentials from HTTP URLs' do
filtered_content = sanitize_url('http://user:pass@test.com/root/repoC.git/')
where(:input, :output) do
'http://user:pass@test.com/root/repoC.git/' | 'http://*****:*****@test.com/root/repoC.git/'
'https://user:pass@test.com/root/repoA.git/' | 'https://*****:*****@test.com/root/repoA.git/'
'ssh://user@host.test/path/to/repo.git' | 'ssh://*****@host.test/path/to/repo.git'
expect(filtered_content).to include("http://*****:*****@test.com/root/repoC.git/")
# git protocol does not support authentication but clean any details anyway
'git://user:pass@host.test/path/to/repo.git' | 'git://*****:*****@host.test/path/to/repo.git'
'git://host.test/path/to/repo.git' | 'git://host.test/path/to/repo.git'
# SCP-style URLs are left unmodified
'user@server:project.git' | 'user@server:project.git'
'user:pass@server:project.git' | 'user:pass@server:project.git'
# return an empty string for invalid URLs
'ssh://' | ''
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
it 'returns an empty string for invalid URLs' do
filtered_content = sanitize_url('ssh://')
expect(filtered_content).to include("repository '' not found")
with_them do
it { expect(sanitize_url(input)).to include("repository '#{output}' not found") }
end
end
describe '.valid?' do
it 'validates url strings' do
expect(described_class.valid?(nil)).to be(false)
expect(described_class.valid?('')).to be(false)
expect(described_class.valid?('valid@project:url.git')).to be(true)
expect(described_class.valid?('123://invalid:url')).to be(false)
where(:value, :url) do
false | nil
false | ''
false | '123://invalid:url'
true | 'valid@project:url.git'
true | 'ssh://example.com'
true | 'ssh://:@example.com'
true | 'ssh://foo@example.com'
true | 'ssh://foo:bar@example.com'
true | 'ssh://foo:bar@example.com/group/group/project.git'
true | 'git://example.com/group/group/project.git'
true | 'git://foo:bar@example.com/group/group/project.git'
true | 'http://foo:bar@example.com/group/group/project.git'
true | 'https://foo:bar@example.com/group/group/project.git'
end
with_them do
it { expect(described_class.valid?(url)).to eq(value) }
end
end
describe '#sanitized_url' do
it { expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") }
context 'credentials in hash' do
where(username: ['foo', '', nil], password: ['bar', '', nil])
with_them do
let(:credentials) { { user: username, password: password } }
subject { described_class.new('http://example.com', credentials: credentials).sanitized_url }
it { is_expected.to eq('http://example.com') }
end
end
context 'credentials in URL' do
where(userinfo: %w[foo:bar@ foo@ :bar@ :@ @] + [nil])
with_them do
subject { described_class.new("http://#{userinfo}example.com").sanitized_url }
it { is_expected.to eq('http://example.com') }
end
end
end
describe '#credentials' do
it { expect(url_sanitizer.credentials).to eq(credentials) }
context 'when user is given to #initialize' do
let(:url_sanitizer) do
described_class.new("https://github.com/me/project.git", credentials: { user: user.username })
context 'credentials in hash' do
where(:input, :output) do
{ user: 'foo', password: 'bar' } | { user: 'foo', password: 'bar' }
{ user: 'foo', password: '' } | { user: 'foo', password: nil }
{ user: 'foo', password: nil } | { user: 'foo', password: nil }
{ user: '', password: 'bar' } | { user: nil, password: 'bar' }
{ user: '', password: '' } | { user: nil, password: nil }
{ user: '', password: nil } | { user: nil, password: nil }
{ user: nil, password: 'bar' } | { user: nil, password: 'bar' }
{ user: nil, password: '' } | { user: nil, password: nil }
{ user: nil, password: nil } | { user: nil, password: nil }
end
it { expect(url_sanitizer.credentials).to eq({ user: 'john.doe' }) }
with_them do
subject { described_class.new('user@example.com:path.git', credentials: input).credentials }
it { is_expected.to eq(output) }
end
it 'overrides URL-provided credentials' do
sanitizer = described_class.new('http://a:b@example.com', credentials: { user: 'c', password: 'd' })
expect(sanitizer.credentials).to eq(user: 'c', password: 'd')
end
end
context 'credentials in URL' do
where(:url, :credentials) do
'http://foo:bar@example.com' | { user: 'foo', password: 'bar' }
'http://:bar@example.com' | { user: nil, password: 'bar' }
'http://foo:@example.com' | { user: 'foo', password: nil }
'http://foo@example.com' | { user: 'foo', password: nil }
'http://:@example.com' | { user: nil, password: nil }
'http://@example.com' | { user: nil, password: nil }
'http://example.com' | { user: nil, password: nil }
# Credentials from SCP-style URLs are not supported at present
'foo@example.com:path' | { user: nil, password: nil }
'foo:bar@example.com:path' | { user: nil, password: nil }
# Other invalid URLs
nil | { user: nil, password: nil }
'' | { user: nil, password: nil }
'no' | { user: nil, password: nil }
end
with_them do
subject { described_class.new(url).credentials }
it { is_expected.to eq(credentials) }
end
end
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
context 'when user is given to #initialize' do
let(:url_sanitizer) do
described_class.new("https://github.com/me/project.git", credentials: { user: user.username })
context 'credentials in hash' do
where(:credentials, :userinfo) do
{ user: 'foo', password: 'bar' } | 'foo:bar@'
{ user: 'foo', password: '' } | 'foo@'
{ user: 'foo', password: nil } | 'foo@'
{ user: '', password: 'bar' } | ':bar@'
{ user: '', password: '' } | nil
{ user: '', password: nil } | nil
{ user: nil, password: 'bar' } | ':bar@'
{ user: nil, password: '' } | nil
{ user: nil, password: nil } | nil
end
it { expect(url_sanitizer.full_url).to eq("https://john.doe@github.com/me/project.git") }
with_them do
subject { described_class.new('http://example.com', credentials: credentials).full_url }
it { is_expected.to eq("http://#{userinfo}example.com") }
end
end
context 'credentials in URL' do
where(:input, :output) do
nil | ''
'' | :same
'git@example.com' | :same
'http://example.com' | :same
'http://foo@example.com' | :same
'http://foo:@example.com' | 'http://foo@example.com'
'http://:bar@example.com' | :same
'http://foo:bar@example.com' | :same
end
with_them do
let(:expected) { output == :same ? input : output }
it { expect(described_class.new(input).full_url).to eq(expected) }
end
end
end
end