Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Merge #6928
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bundlerbot and jdliss committed Apr 28, 2019
2 parents 96b9f63 + 74342c6 commit 807f32e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 3 deletions.
1 change: 1 addition & 0 deletions lib/bundler/fetcher/downloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def fetch(uri, headers = {}, counter = 0)
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)}"
Expand Down
1 change: 1 addition & 0 deletions lib/bundler/fetcher/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def specs(_gem_names)
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
Expand Down
9 changes: 9 additions & 0 deletions spec/bundler/fetcher/downloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@
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
Expand Down
23 changes: 20 additions & 3 deletions spec/bundler/fetcher/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,26 @@
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

Expand Down

0 comments on commit 807f32e

Please sign in to comment.