Skip to content

Commit

Permalink
Fixes #16909 - properly escape user and pass in proxy auth
Browse files Browse the repository at this point in the history
rest-client alongside URIs parse method introduces a bug
where by usernames or passwords cannot have symbols in them.
More Information:
 rest-client/rest-client#661
 ManageIQ/manageiq#17318
  • Loading branch information
jlsherrill committed Feb 12, 2019
1 parent 6720091 commit 750d247
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 4 deletions.
19 changes: 15 additions & 4 deletions app/lib/katello/util/http_proxy.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
require 'katello/util/proxy_uri'

module Katello
module Util
module HttpProxy
def proxy_uri
URI("#{proxy_scheme}://#{proxy_user_info}@#{proxy_host}:#{proxy_port}").to_s if proxy_host
end
#Reset the scheme to proxy(s) based on http or https to handle cgi unescaping in rest-client
# this relies on katello/util/proxy_uri
if proxy_host
scheme = 'proxy' if proxy_scheme == 'http'
scheme = 'proxys' if proxy_scheme == 'https'

uri = URI("#{scheme}://#{proxy_host}:#{proxy_port}")
if proxy_config && proxy_config[:user]
uri.user = CGI.escape(proxy_config[:user])
uri.password = CGI.escape(proxy_config[:password])
end

def proxy_user_info
"#{proxy_config[:user]}:#{proxy_config[:password]}" if proxy_config && proxy_config[:user]
uri.to_s
end
end

def proxy_config
Expand Down
64 changes: 64 additions & 0 deletions app/lib/katello/util/proxy_uri.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#
# The Ruby URI parser doesn't decode the percent encoded characters in the URI, in particular it
# doesn't decode the password which is frequently used when specifying proxy addresses and
# authentication. For example, the following code:
#
# require 'uri'
# proxy = URI.parse('http://myuser:%24%[email protected]:3128')
# puts proxy.password
#
# Produces the following output:
#
# %24%3fxxxx
#
# But some gems, in particular `rest-client` and `kubeclient`, expect it to decode those characters,
# as they use the value returned by the `password` method directly, and thus they fail to
# authenticate against the proxy server when the password contains percent encoded characters.
#
# To address this issue this file adds a new `proxy` URI schema that almost identical to the `http`
# schema, but that decodes the password before returning it. Users can use this schema instead of
# `http` when they need to use percent encoded characters in the password. For example, the user
# can type in the UI the following proxy URL:
#
# proxy://myuser:%24%[email protected]:3128
#
# And the new schema will automatically translate `%24%3fxxxx` into `$?xxxx`.
# This fix is derived from: https://github.com/ManageIQ/manageiq/pull/17318
require 'cgi'
require 'uri'

module URI
class ProxyUri < HTTP
def password
value = super
value = CGI.unescape(value) if value
value
end

def user
value = super
value = CGI.unescape(value) if value
value
end
end

@@schemes['PROXY'] = ProxyUri
end

module URI
class ProxysUri < HTTP
def password
value = super
value = CGI.unescape(value) if value
value
end

def user
value = super
value = CGI.unescape(value) if value
value
end
end

@@schemes['PROXYS'] = ProxysUri
end
31 changes: 31 additions & 0 deletions test/lib/util/http_proxy_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
require 'katello_test_helper'

module Katello
module Util
class HttpProxyTest < ActiveSupport::TestCase
include Katello::Util::HttpProxy

def test_handles_no_username_test
SETTINGS[:katello][:cdn_proxy] = {
host: 'http://foobar.com',
username: nil,
password: nil
}
assert_equal 'proxy://foobar.com', proxy_uri
end

def test_properly_escapes_username
SETTINGS[:katello][:cdn_proxy] = {
host: 'http://foobar.com',
user: 'red!hat',
password: 'red@hat'
}
assert_equal 'proxy://red%21hat:red%[email protected]', proxy_uri

uri = URI.parse(proxy_uri)
assert_equal 'red!hat', uri.user
assert_equal 'red@hat', uri.password
end
end
end
end

0 comments on commit 750d247

Please sign in to comment.