Skip to content

Commit

Permalink
Move to explicit method call for Basic Auth credentials in push client
Browse files Browse the repository at this point in the history
While URLs do support passing Basic Auth credentials using the
`http://user:password@example.com/` syntax, the username and password in
that syntax have to follow the usual rules for URL encoding of
characters per RFC 3986
(https://datatracker.ietf.org/doc/html/rfc3986#section-2.1).

Rather than place the burden of correctly performing that encoding on
users of this gem, we decided to have a separate method for supplying
Basic Auth credentials, with no requirement to URL encode the characters
in them.
  • Loading branch information
Sinjo committed Jan 9, 2022
1 parent 391aa20 commit 2ae084f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
30 changes: 29 additions & 1 deletion lib/prometheus/client/push.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,21 @@ def initialize(job:, gateway: DEFAULT_GATEWAY, grouping_key: {}, **kwargs)
@gateway = gateway || DEFAULT_GATEWAY
@grouping_key = grouping_key
@path = build_path(job, grouping_key)

@uri = parse("#{@gateway}#{@path}")
validate_no_basic_auth!(@uri)

@http = Net::HTTP.new(@uri.host, @uri.port)
@http.use_ssl = (@uri.scheme == 'https')
@http.open_timeout = kwargs[:open_timeout] if kwargs[:open_timeout]
@http.read_timeout = kwargs[:read_timeout] if kwargs[:read_timeout]
end

def basic_auth(user, password)
@user = user
@password = password
end

def add(registry)
synchronize do
request(Net::HTTP::Post, registry)
Expand Down Expand Up @@ -113,7 +120,7 @@ def request(req_class, registry = nil)

req = req_class.new(@uri)
req.content_type = Formats::Text::CONTENT_TYPE
req.basic_auth(@uri.user, @uri.password) if @uri.user
req.basic_auth(@user, @password) if @user
req.body = Formats::Text.marshal(registry) if registry

response = @http.request(req)
Expand All @@ -126,6 +133,27 @@ def synchronize
@mutex.synchronize { yield }
end

def validate_no_basic_auth!(uri)
if uri.user || uri.password
raise ArgumentError, <<~EOF
Setting Basic Auth credentials in the gateway URL is not supported, please call the `basic_auth` method
Received username `#{uri.user}` in gateway URL
While URLs do support passing Basic Auth credentials using the
`http://user:password@example.com/` syntax, the username and
password in that syntax have to follow the usual rules for URL
encoding of characters per RFC 3986
(https://datatracker.ietf.org/doc/html/rfc3986#section-2.1).
Rather than place the burden of correctly performing that encoding
on users of this gem, we decided to have a separate method for
supplying Basic Auth credentials, with no requirement to URL encode
the characters in them.
EOF
end
end

def validate_no_label_clashes!(registry)
# There's nothing to check if we don't have a grouping key
return if @grouping_key.empty?
Expand Down
40 changes: 26 additions & 14 deletions spec/prometheus/client/push_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,23 +271,35 @@
end

context 'Basic Auth support' do
let(:gateway) { 'https://super:secret@localhost:9091' }
context 'when credentials are passed in the gateway URL' do
let(:gateway) { 'https://super:secret@localhost:9091' }

it 'sets Basic Auth header when requested' do
request = double(:request)
expect(request).to receive(:content_type=).with(content_type)
expect(request).to receive(:basic_auth).with('super', 'secret')
expect(request).to receive(:body=).with(data)
expect(Net::HTTP::Put).to receive(:new).with(uri).and_return(request)
it "raises an ArgumentError explaining why we don't support that mechanism" do
expect { push }.to raise_error ArgumentError, /in the gateway URL.*username `super`/m
end
end

http = double(:http)
expect(http).to receive(:use_ssl=).with(true)
expect(http).to receive(:open_timeout=).with(5)
expect(http).to receive(:read_timeout=).with(30)
expect(http).to receive(:request).with(request).and_return(response)
expect(Net::HTTP).to receive(:new).with('localhost', 9091).and_return(http)
context 'when credentials are passed to the separate `basic_auth` method' do
let(:gateway) { 'https://localhost:9091' }

it 'passes the credentials on to the HTTP client' do
request = double(:request)
expect(request).to receive(:content_type=).with(content_type)
expect(request).to receive(:basic_auth).with('super', 'secret')
expect(request).to receive(:body=).with(data)
expect(Net::HTTP::Put).to receive(:new).with(uri).and_return(request)

http = double(:http)
expect(http).to receive(:use_ssl=).with(true)
expect(http).to receive(:open_timeout=).with(5)
expect(http).to receive(:read_timeout=).with(30)
expect(http).to receive(:request).with(request).and_return(response)
expect(Net::HTTP).to receive(:new).with('localhost', 9091).and_return(http)

push.basic_auth("super", "secret")

push.send(:request, Net::HTTP::Put, registry)
push.send(:request, Net::HTTP::Put, registry)
end
end
end

Expand Down

0 comments on commit 2ae084f

Please sign in to comment.