From ab7f65233e51e43c983c88f726efcd3d7a39f40d Mon Sep 17 00:00:00 2001 From: Robb Manes Date: Wed, 28 Mar 2018 10:46:42 -0400 Subject: [PATCH 1/6] Resolve string handling for "https://" or "http://" in update_rhsm_conf Previously, if a proxy URI was entered into the registration page leading with "https://" or "http://", it would be incorrectly entered into the rhsm.conf. This patch prevents this by ensuring it is a valid URI, and correctly parsing the string regardless if it has only a host in the URI or if it leads with the protocol. https://bugzilla.redhat.com/show_bug.cgi?id=1561448 --- app/models/registration_system.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/registration_system.rb b/app/models/registration_system.rb index 182d4f384d7..a8250d942d9 100644 --- a/app/models/registration_system.rb +++ b/app/models/registration_system.rb @@ -83,8 +83,9 @@ def self.update_rhsm_conf(options = {}) return unless option_values[:proxy_address] - write_rhsm_config(:proxy_hostname => option_values[:proxy_address].split(':')[0], - :proxy_port => option_values[:proxy_address].split(':')[1], + proxy_uri = option_values[:proxy_address].gsub(/http(|s):\/\//, "") + write_rhsm_config(:proxy_hostname => proxy_uri.split(':')[0], + :proxy_port => proxy_uri.split(':')[1], :proxy_user => option_values[:proxy_username], :proxy_password => option_values[:proxy_password]) end From 58c65e44d21368ce9ddd49042a978c0d9603ab81 Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Thu, 29 Mar 2018 14:15:07 -0400 Subject: [PATCH 2/6] Add tests for RegistrationSystem.update_rhsm_conf with http and https https://bugzilla.redhat.com/show_bug.cgi?id=1561448 --- spec/models/registration_system_spec.rb | 31 +++++++++++++++++-------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/spec/models/registration_system_spec.rb b/spec/models/registration_system_spec.rb index e2f7384b5d6..a26ae4e966d 100644 --- a/spec/models/registration_system_spec.rb +++ b/spec/models/registration_system_spec.rb @@ -195,14 +195,9 @@ EOT end - let(:rhsm_conf) { Tempfile.new(@spec_name.downcase) } + let(:rhsm_conf) { Tempfile.new } before do - @proxy_args = {:registration_http_proxy_server => "192.0.2.0:myport", - :registration_http_proxy_username => "my_dummy_username", - :registration_http_proxy_password => "my_dummy_password"} - - @spec_name = File.basename(__FILE__).split(".rb").first.freeze stub_const("RegistrationSystem::RHSM_CONFIG_FILE", rhsm_conf.path) rhsm_conf.write(original_rhsm_conf) rhsm_conf.close @@ -213,10 +208,26 @@ FileUtils.rm_f("#{rhsm_conf.path}.miq_orig") end - it "will save then update the original config file" do - RegistrationSystem.update_rhsm_conf(@proxy_args) - expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) - expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + context "will save then update the original config file" do + let(:params) { {:registration_http_proxy_username => "my_dummy_username", :registration_http_proxy_password => "my_dummy_password"} } + + it "with server:port" do + RegistrationSystem.update_rhsm_conf(params.merge(:registration_http_proxy_server => "192.0.2.0:myport")) + expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) + expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + end + + it "with http://server:port" do + RegistrationSystem.update_rhsm_conf(params.merge(:registration_http_proxy_server => "http://192.0.2.0:myport")) + expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) + expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + end + + it "with https://server:port" do + RegistrationSystem.update_rhsm_conf(params.merge(:registration_http_proxy_server => "https://192.0.2.0:myport")) + expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) + expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + end end it "with no proxy server will not update the rhsm config file" do From 65bfb6a12816774fbc532ca2581533a1917d495b Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Thu, 29 Mar 2018 14:21:50 -0400 Subject: [PATCH 3/6] Reduce duplicate test logic https://bugzilla.redhat.com/show_bug.cgi?id=1561448 --- spec/models/registration_system_spec.rb | 30 ++++++++++--------------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/spec/models/registration_system_spec.rb b/spec/models/registration_system_spec.rb index a26ae4e966d..e7d382ac558 100644 --- a/spec/models/registration_system_spec.rb +++ b/spec/models/registration_system_spec.rb @@ -209,24 +209,18 @@ end context "will save then update the original config file" do - let(:params) { {:registration_http_proxy_username => "my_dummy_username", :registration_http_proxy_password => "my_dummy_password"} } - - it "with server:port" do - RegistrationSystem.update_rhsm_conf(params.merge(:registration_http_proxy_server => "192.0.2.0:myport")) - expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) - expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) - end - - it "with http://server:port" do - RegistrationSystem.update_rhsm_conf(params.merge(:registration_http_proxy_server => "http://192.0.2.0:myport")) - expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) - expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) - end - - it "with https://server:port" do - RegistrationSystem.update_rhsm_conf(params.merge(:registration_http_proxy_server => "https://192.0.2.0:myport")) - expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) - expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + ["", "http://", "https://"].each do |prefix| + params = { + :registration_http_proxy_server => "#{prefix}192.0.2.0:myport", + :registration_http_proxy_username => "my_dummy_username", + :registration_http_proxy_password => "my_dummy_password" + } + + it "with #{params[:registration_http_proxy_server]}" do + RegistrationSystem.update_rhsm_conf(params) + expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) + expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + end end end From 47ddbb4a967fed9ae31028088efe97896960d72b Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Thu, 29 Mar 2018 14:28:28 -0400 Subject: [PATCH 4/6] Test against multiple addresses (hostname, IPv4, IPv6) https://bugzilla.redhat.com/show_bug.cgi?id=1561448 --- spec/models/registration_system_spec.rb | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/spec/models/registration_system_spec.rb b/spec/models/registration_system_spec.rb index e7d382ac558..5a8645ce74b 100644 --- a/spec/models/registration_system_spec.rb +++ b/spec/models/registration_system_spec.rb @@ -181,7 +181,7 @@ ssl_verify_depth = 3 # an http proxy server to use - proxy_hostname = 192.0.2.0 + proxy_hostname = ProxyHostnameValue # port for http proxy server proxy_port = myport @@ -210,16 +210,18 @@ context "will save then update the original config file" do ["", "http://", "https://"].each do |prefix| - params = { - :registration_http_proxy_server => "#{prefix}192.0.2.0:myport", - :registration_http_proxy_username => "my_dummy_username", - :registration_http_proxy_password => "my_dummy_password" - } - - it "with #{params[:registration_http_proxy_server]}" do - RegistrationSystem.update_rhsm_conf(params) - expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) - expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf) + ["proxy.example.com", "192.0.2.0", "[2001:db8::]"].each do |address| + params = { + :registration_http_proxy_server => "#{prefix}#{address}:myport", + :registration_http_proxy_username => "my_dummy_username", + :registration_http_proxy_password => "my_dummy_password" + } + + it "with #{params[:registration_http_proxy_server]}" do + RegistrationSystem.update_rhsm_conf(params) + expect(File.read("#{rhsm_conf.path}.miq_orig")).to eq(original_rhsm_conf) + expect(File.read(rhsm_conf)).to eq(updated_rhsm_conf.sub(/ProxyHostnameValue/, address)) + end end end end From dafb5f7a99bb0409191a58f03bd55b59729e9cbb Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Thu, 29 Mar 2018 15:16:39 -0400 Subject: [PATCH 5/6] myport is not a valid port, 0 is reserved so use that instead https://bugzilla.redhat.com/show_bug.cgi?id=1561448 --- spec/models/registration_system_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/registration_system_spec.rb b/spec/models/registration_system_spec.rb index 5a8645ce74b..30f2e4fa911 100644 --- a/spec/models/registration_system_spec.rb +++ b/spec/models/registration_system_spec.rb @@ -184,7 +184,7 @@ proxy_hostname = ProxyHostnameValue # port for http proxy server - proxy_port = myport + proxy_port = 0 # user name for authenticating to an http proxy, if needed proxy_user = my_dummy_username @@ -212,7 +212,7 @@ ["", "http://", "https://"].each do |prefix| ["proxy.example.com", "192.0.2.0", "[2001:db8::]"].each do |address| params = { - :registration_http_proxy_server => "#{prefix}#{address}:myport", + :registration_http_proxy_server => "#{prefix}#{address}:0", :registration_http_proxy_username => "my_dummy_username", :registration_http_proxy_password => "my_dummy_password" } @@ -234,10 +234,10 @@ it "with no options will use database valuses" do MiqDatabase.seed MiqDatabase.first.update_attributes( - :registration_http_proxy_server => "192.0.2.0:myport" + :registration_http_proxy_server => "192.0.2.0:0" ) RegistrationSystem.update_rhsm_conf - expect(File.read(rhsm_conf)).to include("proxy_hostname = 192.0.2.0", "proxy_port = myport") + expect(File.read(rhsm_conf)).to include("proxy_hostname = 192.0.2.0", "proxy_port = 0") end end end From 963a3f3a6c2a5d2b5f1059b06c75756323f22a13 Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Thu, 29 Mar 2018 15:19:09 -0400 Subject: [PATCH 6/6] Fix issues related to proxy_address - Allow for http(s) or other schemes - Allow for hostname, IPv4 or IPv6 addresses https://bugzilla.redhat.com/show_bug.cgi?id=1561448 --- app/models/registration_system.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/registration_system.rb b/app/models/registration_system.rb index a8250d942d9..186a9c82fbb 100644 --- a/app/models/registration_system.rb +++ b/app/models/registration_system.rb @@ -83,9 +83,9 @@ def self.update_rhsm_conf(options = {}) return unless option_values[:proxy_address] - proxy_uri = option_values[:proxy_address].gsub(/http(|s):\/\//, "") - write_rhsm_config(:proxy_hostname => proxy_uri.split(':')[0], - :proxy_port => proxy_uri.split(':')[1], + proxy_uri = URI.parse(option_values[:proxy_address].include?("://") ? option_values[:proxy_address] : "http://#{option_values[:proxy_address]}") + write_rhsm_config(:proxy_hostname => proxy_uri.host, + :proxy_port => proxy_uri.port, :proxy_user => option_values[:proxy_username], :proxy_password => option_values[:proxy_password]) end