tmatilai / vagrant-proxyconf

Vagrant plugin that configures the virtual machine to use proxies
MIT License
531 stars 74 forks source link

Add uri encoded user password #130

Closed otahi closed 8 years ago

otahi commented 9 years ago

To solve the issue "Yum config and URL-encoded auth" #129. Following code set URI encoded user name and password to yum or chef.

if Vagrant.has_plugin?("vagrant-proxyconf")
    config.proxy.http  = "http://user:123%23abc@proxy.example.com:80"
    config.proxy.uri_encoded = true
end

=> user: user, password: 123#abc

otahi commented 9 years ago

I have tested on my CentOS7 VM, then it seems OK.

@awiseman Can you check with your environment with this. See The way to install development version.

My test is as follows.

Vagrant.configure("2") do |config|
  config.vm.box = "hfm4/centos7"
  config.proxy.http    = "http://user:123%23abc@proxy.example.com:80"
  config.proxy.uri_encoded = true
end

The result.

[vagrant@localhost ~]$ cat /etc/yum.conf | grep proxy
proxy=http://proxy.example.com:80
proxy_username=user
proxy_password=123#abc
[vagrant@localhost ~]$
awiseman commented 9 years ago

Looks good to me. I ran a few tests in my OEL 6.5 VM:

uri_encoded = true,  password = 123%23abc  # should output 123#abc
uri_encoded = false, password = 123%23abc  # should output 123%23abc
uri_encoded = true,  password = 123_abc    # should output 123_abc
uri_encoded = false, password = 123_abc    # should output 123_abc

Each produced the expected result.

The second case gave me a thought, however: Is there ever going to be a case where the value of the http key is not URI encoded? Would it simplify the API if we just assumed that it always is? I don't think the second test would produce the desired behavior, overall, since the user would need to URI encode the % character in order to get a working literal for the http_proxy environment variable.

otahi commented 9 years ago

@awiseman Thank you for you testing and sharing your thought. I think your indication is reasonable. I thought the following tradeoff.

Now, I am changing my mind to care usability.

@tmatilai What do you think? Is it acceptable for you not to care above case? If so, it will be simpler for users.

otahi commented 9 years ago

@awiseman I have changed my mind. I added URI encoded username and password in general. The original commits can be found on https://github.com/tmatilai/vagrant-proxyconf/compare/022494...otahi:add_uri_encoded_option. Can you check this change?

@tmatilai Do you have any indications?

awiseman commented 9 years ago

Sorry for the delay, I tested the new add_uri_encoded_user_password branch and it looks good to me.

Thanks for your work on this!

tmatilai commented 8 years ago

Looks good to me, too. =)

otahi commented 8 years ago

Thank you for checking! @tmatilai If possible, please release last 2 modifications.

otahi commented 8 years ago

@tmatilai I have added last 2 modifications to CHANGELOG. Can you release 1.5.2?

tmatilai commented 8 years ago

Thanks a lot @otahi! I'll cut the release today.

tmatilai commented 8 years ago

It's still today, no? Anyway, 1.5.2 was just released. Thanks to all!

otahi commented 8 years ago

@tmatilai Thank you for the release!! I updated my plugin, and it works well. Don't care about the date :sunrise: !