Passing a URL with embedded basic auth is broken
iMacTia opened this issue ยท 6 comments
The recent authentication middleware refactoring broke passing a URL with embedded basic auth in the user info part of the URI, because basic_auth is still used in url_prefix=
faraday/lib/faraday/connection.rb
Lines 364 to 367 in 0f9626c
~/src/github.com/lostisland/faraday (1.x) $ ruby -Ilib -rfaraday -rfaraday/net_http -rjson -e "puts JSON.load(Faraday.new(url: 'https://user:password@httpbin.org/headers').get.body)['headers']['Authorization']"
WARNING: `Faraday::Connection#basic_auth` is deprecated; it will be removed in version 2.0.
While initializing your connection, use `#request(:basic_auth, ...)` instead.
See https://lostisland.github.io/faraday/middleware/authentication for more usage info.
Basic dXNlcjpwYXNzd29yZA==
~/src/github.com/lostisland/faraday (main) $ ruby -Ilib -rfaraday -rfaraday/net_http -rjson -e "puts JSON.load(Faraday.new(url: 'https://user:password@httpbin.org/headers').get.body)['headers']['Authorization']"
lib/faraday/connection.rb:365:in `block in url_prefix=': undefined method `basic_auth' for #<Faraday::Connection:0x00007fed250abaa8 @parallel_manager=nil, @headers={}, @params={}, @options=#<Faraday::RequestOptions (empty)>, @ssl=#<Faraday::SSLOptions (empty)>, @default_parallel_manager=nil, @manual_proxy=nil, @builder=#<Faraday::RackBuilder:0x00007fed25092210 @adapter=Faraday::Adapter::NetHttp, @handlers=[Faraday::Request::UrlEncoded]>, @url_prefix=#<URI::HTTPS https://user:password@httpbin.org/headers>> (NoMethodError)
from lib/faraday/connection.rb:506:in `with_uri_credentials'
from lib/faraday/connection.rb:364:in `url_prefix='
from lib/faraday/connection.rb:84:in `initialize'
from lib/faraday.rb:96:in `new'
from lib/faraday.rb:96:in `new'
from -e:1:in `<main>'
A test demonstrating this is available here: 1.x...etiennebarrie:test-basic-auth-in-url:
it 'uses User Information from the URI for Basic authentication' do
conn.url_prefix = 'http://user:password@sushi.com'
expect(conn.url_prefix.to_s).to eq('http://sushi.com/')
request = conn.build_request(:get)
expect(request.headers['Authorization']).to eq("Basic #{Base64.strict_encode64('user:password')}")
end
We should decide if we want to fix this or remove support for this feature.
See comment by @etiennebarrie in #1308 (comment)_
@etiennebarrie I'm quoting your thoughts from the comment:
While I'm not against deprecating basic_auth and authorization on Connection, I think being able to have basic auth embedded in the URL is really useful (it can be configured in secrets for example).
I'm seriously thinking of removing the support for this.
It seems like this practice have been deprecated back in 2005 and most of the major browsers have been dropping its support as well.
I'm also curious about your example above:
it can be configured in secrets for example
I don't see how using thebasic_auth
middleware would not allow to use secrets?
Wouldn't something like this work?
# Faraday 1.x
conn = Faraday.new(url_with_no_userinfo) do |f|
conn.request :basic_auth, Secrets.basic_auth_user, Secrets.basic_auth_pass
...
end
We basically do:
def connection
Faraday.new(url: server) do
# other config
end
end
def server
if global?
secrets.global
elsif something?
secrets.other
elsif something_else?
secrets.another
end
end
So we just need one secret per URL/user/password.
We can totally split each secret into three, or even keep the single URL secret but extract the user password before passing it down to Faraday.
Yes that's what I thought, thanks for confirming @etiennebarrie ๐
Hi, here it is another use case: elastic/elasticsearch-ruby#1479
Thanks for the input @tagliala, ultimately we want to please the community, so it's important to understand how much this feature is used in order to decide about its future.
This is obviously a widespread library, so many thanks for pointing it out
@etiennebarrie main
branch is now fixed and supports passing basic auth as part of the url again.
I'm also happy to confirm that I'm not currently planning on removing this feature anymore as the maintenance burden is quite small ๐.
@tagliala the deprecation warning has also been fixed and I've just released v1.7.2 ๐