New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Passing a URL with embedded basic auth is broken #1319
Comments
url_prefix=
https://github.com/lostisland/faraday/blob/0f9626c48d0daa24888cb4e5e7962c106a48d97f/lib/faraday/connection.rb#L364-L367
@etiennebarrie I'm quoting your thoughts from the comment:
I'm seriously thinking of removing the support for this. I'm also curious about your example above:
# 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 @tagliala the deprecation warning has also been fixed and I've just released v1.7.2 🙌 |
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
A test demonstrating this is available here: 1.x...etiennebarrie:test-basic-auth-in-url:
We should decide if we want to fix this or remove support for this feature.
See comment by @etiennebarrie in #1308 (comment)_
The text was updated successfully, but these errors were encountered: