diff --git a/lib/prometheus/client/push.rb b/lib/prometheus/client/push.rb index 04f4d7ae..6d1aa8ae 100644 --- a/lib/prometheus/client/push.rb +++ b/lib/prometheus/client/push.rb @@ -39,7 +39,9 @@ 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') @@ -47,6 +49,11 @@ def initialize(job:, gateway: DEFAULT_GATEWAY, grouping_key: {}, **kwargs) @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) @@ -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) @@ -126,6 +133,39 @@ 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. Instead of passing + Basic Auth credentials like this: + + ``` + push = Prometheus::Client::Push.new(job: "my-job", gateway: "http://user:password@localhost:9091") + ``` + + please pass them like this instead: + + ``` + push = Prometheus::Client::Push.new(job: "my-job", gateway: "http://localhost:9091") + push.basic_auth("user", "password") + ``` + + 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? diff --git a/spec/prometheus/client/push_spec.rb b/spec/prometheus/client/push_spec.rb index 372094ca..e4fede99 100644 --- a/spec/prometheus/client/push_spec.rb +++ b/spec/prometheus/client/push_spec.rb @@ -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