diff options
author | Bundlerbot <bot@bundler.io> | 2019-04-28 14:31:13 +0000 |
---|---|---|
committer | Bundlerbot <bot@bundler.io> | 2019-04-28 14:31:13 +0000 |
commit | 807f32e8b7e6eec92325c1a8af1daa2b4983b85d (patch) | |
tree | 9c37aea6294e1ef62499417eb9859044c557a18b | |
parent | 96b9f632a758cc06abe3c17e8623dfba09e3ee7f (diff) | |
parent | 74342c6c9e0cca5101de22a85c9367b5969ec46a (diff) | |
download | bundler-807f32e8b7e6eec92325c1a8af1daa2b4983b85d.tar.gz |
Merge #6928
6928: When a 401 response is received, raise BadAuthenticationError if credentials were present in the request URI r=deivid-rodriguez a=jdliss
### What was the end-user problem that led to this PR?
Some gem servers respond to bad username/password request with a 401 instead of a 403.
### What was your diagnosis of the problem?
When a 401 response was received, bundler would automatically assume no credentials were supplied, leading to a response of
```
Authentication is required for http://moto@gems.motologic.com/.
Please supply credentials for this source. You can do this by running:
bundle config http://moto@gems.motologic.com/ username:password
```
even when a username and password was present in the bundler config.
### What is your fix for the problem, implemented in this PR?
There already exists a check for credentials in the request URI for 403 responses. I took that pattern and implemented it for 401 responses as well.
### Why did you choose this fix out of the possible options?
I chose this fix because a very similar pattern already exists in bundler, I just applied the same logic to another response code.
Co-authored-by: Jonathan <jonacom@lissismore.com>
-rw-r--r-- | lib/bundler/fetcher/downloader.rb | 1 | ||||
-rw-r--r-- | lib/bundler/fetcher/index.rb | 1 | ||||
-rw-r--r-- | spec/bundler/fetcher/downloader_spec.rb | 9 | ||||
-rw-r--r-- | spec/bundler/fetcher/index_spec.rb | 23 |
4 files changed, 31 insertions, 3 deletions
diff --git a/lib/bundler/fetcher/downloader.rb b/lib/bundler/fetcher/downloader.rb index 2aeb9962c4..73f125af91 100644 --- a/lib/bundler/fetcher/downloader.rb +++ b/lib/bundler/fetcher/downloader.rb @@ -37,6 +37,7 @@ module Bundler when Net::HTTPTooManyRequests raise TooManyRequestsError, response.body when Net::HTTPUnauthorized + raise BadAuthenticationError, uri.host if uri.userinfo raise AuthenticationRequiredError, uri.host when Net::HTTPNotFound raise FallbackError, "Net::HTTPNotFound: #{URICredentialsFilter.credential_filtered_uri(uri)}" diff --git a/lib/bundler/fetcher/index.rb b/lib/bundler/fetcher/index.rb index e7baf63873..9beb0e27d8 100644 --- a/lib/bundler/fetcher/index.rb +++ b/lib/bundler/fetcher/index.rb @@ -13,6 +13,7 @@ module Bundler when /certificate verify failed/ raise CertificateFailureError.new(display_uri) when /401/ + raise BadAuthenticationError, remote_uri if remote_uri.userinfo raise AuthenticationRequiredError, remote_uri when /403/ raise BadAuthenticationError, remote_uri if remote_uri.userinfo diff --git a/spec/bundler/fetcher/downloader_spec.rb b/spec/bundler/fetcher/downloader_spec.rb index 07b507266b..f985b88982 100644 --- a/spec/bundler/fetcher/downloader_spec.rb +++ b/spec/bundler/fetcher/downloader_spec.rb @@ -82,6 +82,15 @@ RSpec.describe Bundler::Fetcher::Downloader do expect { subject.fetch(uri, options, counter) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, /Authentication is required for www.uri-to-fetch.com/) end + + context "when the there are credentials provided in the request" do + let(:uri) { URI("http://user:password@www.uri-to-fetch.com") } + + it "should raise a Bundler::Fetcher::BadAuthenticationError that doesn't contain the password" do + expect { subject.fetch(uri, options, counter) }. + to raise_error(Bundler::Fetcher::BadAuthenticationError, /Bad username or password for www.uri-to-fetch.com/) + end + end end context "when the request response is a Net::HTTPNotFound" do diff --git a/spec/bundler/fetcher/index_spec.rb b/spec/bundler/fetcher/index_spec.rb index 0cf0ae764e..d5ededae3e 100644 --- a/spec/bundler/fetcher/index_spec.rb +++ b/spec/bundler/fetcher/index_spec.rb @@ -35,9 +35,26 @@ RSpec.describe Bundler::Fetcher::Index do context "when a 401 response occurs" do let(:error_message) { "401" } - it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do - expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, - %r{Authentication is required for http://remote-uri.org}) + before do + allow(remote_uri).to receive(:userinfo).and_return(userinfo) + end + + context "and there was userinfo" do + let(:userinfo) { double(:userinfo) } + + it "should raise a Bundler::Fetcher::BadAuthenticationError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::BadAuthenticationError, + %r{Bad username or password for http://remote-uri.org}) + end + end + + context "and there was no userinfo" do + let(:userinfo) { nil } + + it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, + %r{Authentication is required for http://remote-uri.org}) + end end end |