Skip to content

Commit

Permalink
fix(security): hide personal access token given in uri (#225)
Browse files Browse the repository at this point in the history
something like https://pat@my-pact-server/pact.json is possible where pat stands for personal access token and is a secret.
fix the current behavior where only https://user:password@my-pact-server/pact.json is checked
  • Loading branch information
thomas-girotto authored Oct 11, 2020
1 parent 3cd57cb commit f6db12d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
15 changes: 13 additions & 2 deletions lib/pact/provider/pact_uri.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def == other
end

def basic_auth?
!!username
!!username && !!password
end

def username
Expand All @@ -29,12 +29,23 @@ def password
end

def to_s
if basic_auth? && uri.start_with?('http://', 'https://')
if basic_auth? && http_or_https_uri?
URI(@uri).tap { |x| x.userinfo="#{username}:*****"}.to_s
elsif personal_access_token? && http_or_https_uri?
URI(@uri).tap { |x| x.userinfo="*****"}.to_s
else
uri
end
end

private def personal_access_token?
!!username && !password
end

private def http_or_https_uri?
uri.start_with?('http://', 'https://')
end

end
end
end
21 changes: 19 additions & 2 deletions spec/lib/pact/provider/pact_uri_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
end

describe '#to_s' do
context 'with userinfo provided' do
context 'with basic auth provided' do
let(:password) { 'my_password' }
let(:options) { { username: username, password: password } }

it 'should include user name and password' do
it 'should include user name and and hide password' do
expect(pact_uri.to_s).to eq('http://pact:*****@uri')
end

Expand All @@ -40,6 +40,23 @@
end
end

context 'with personal access token provided' do
let(:pat) { 'should_be_secret' }
let(:options) { { username: pat } }

it 'should hide the pat' do
expect(pact_uri.to_s).to eq('http://*****@uri')
end

context 'when pat credentials have been set for a local file (eg. via environment variables, unintentionally)' do
let(:uri) { '/some/file thing.json' }

it 'does not blow up' do
expect(pact_uri.to_s).to eq uri
end
end
end

context 'without userinfo' do
let(:options) { {} }

Expand Down

0 comments on commit f6db12d

Please sign in to comment.