From fe240b672b709e800f2b4fb996f04f2005329dab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Thu, 10 Aug 2023 21:37:15 +0200 Subject: [PATCH] [rubygems/rubygems] Show better error when PAT can't authenticate to a private server Before: ``` Fetching gem metadata from https://rubygems.org/........ Fetching source index from https://rubygems.pkg.github.com/my-org/ Bad username or password for https://x-access-token@rubygems.pkg.github.com/my-org/. Please double-check your credentials and correct them. ``` After: ``` Fetching gem metadata from https://rubygems.org/........ Fetching source index from https://rubygems.pkg.github.com/my-org/ Access token could not be authenticated for https://x-access-token@rubygems.pkg.github.com/my-org/. Make sure it's valid and has the necessary scopes configured. ``` https://github.com/rubygems/rubygems/commit/2ae69c964a --- lib/bundler/fetcher.rb | 12 +++++++++- lib/bundler/fetcher/downloader.rb | 2 ++ lib/bundler/fetcher/index.rb | 3 +-- .../bundler/fetcher/downloader_spec.rb | 10 ++++++++ spec/bundler/bundler/fetcher/index_spec.rb | 23 +++---------------- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/lib/bundler/fetcher.rb b/lib/bundler/fetcher.rb index e12c15af8a8a24..2119799f688437 100644 --- a/lib/bundler/fetcher.rb +++ b/lib/bundler/fetcher.rb @@ -61,6 +61,16 @@ def initialize(remote_uri) end end + # This error is raised if HTTP authentication is correct, but lacks + # necessary permissions. + class AuthenticationForbiddenError < HTTPError + def initialize(remote_uri) + remote_uri = filter_uri(remote_uri) + super "Access token could not be authenticated for #{remote_uri}.\n" \ + "Make sure it's valid and has the necessary scopes configured." + end + end + # Exceptions classes that should bypass retry attempts. If your password didn't work the # first time, it's not going to the third time. NET_ERRORS = [:HTTPBadGateway, :HTTPBadRequest, :HTTPFailedDependency, @@ -70,7 +80,7 @@ def initialize(remote_uri) :HTTPRequestURITooLong, :HTTPUnauthorized, :HTTPUnprocessableEntity, :HTTPUnsupportedMediaType, :HTTPVersionNotSupported].freeze FAIL_ERRORS = begin - fail_errors = [AuthenticationRequiredError, BadAuthenticationError, FallbackError] + fail_errors = [AuthenticationRequiredError, BadAuthenticationError, AuthenticationForbiddenError, FallbackError] fail_errors << Gem::Requirement::BadRequirementError fail_errors.concat(NET_ERRORS.map {|e| Net.const_get(e) }) end.freeze diff --git a/lib/bundler/fetcher/downloader.rb b/lib/bundler/fetcher/downloader.rb index 28d33f1aed3cbb..3062899e0e7a90 100644 --- a/lib/bundler/fetcher/downloader.rb +++ b/lib/bundler/fetcher/downloader.rb @@ -41,6 +41,8 @@ def fetch(uri, headers = {}, counter = 0) when Net::HTTPUnauthorized raise BadAuthenticationError, uri.host if uri.userinfo raise AuthenticationRequiredError, uri.host + when Net::HTTPForbidden + raise AuthenticationForbiddenError, uri.host when Net::HTTPNotFound raise FallbackError, "Net::HTTPNotFound: #{filtered_uri}" else diff --git a/lib/bundler/fetcher/index.rb b/lib/bundler/fetcher/index.rb index 6bb9fcc1939388..c623647f01cdcc 100644 --- a/lib/bundler/fetcher/index.rb +++ b/lib/bundler/fetcher/index.rb @@ -15,8 +15,7 @@ def specs(_gem_names) raise BadAuthenticationError, remote_uri if remote_uri.userinfo raise AuthenticationRequiredError, remote_uri when /403/ - raise BadAuthenticationError, remote_uri if remote_uri.userinfo - raise AuthenticationRequiredError, remote_uri + raise AuthenticationForbiddenError, remote_uri else raise HTTPError, "Could not fetch specs from #{display_uri} due to underlying error <#{e.message}>" end diff --git a/spec/bundler/bundler/fetcher/downloader_spec.rb b/spec/bundler/bundler/fetcher/downloader_spec.rb index 0412ddb83a3a54..22aa3a8b0c6809 100644 --- a/spec/bundler/bundler/fetcher/downloader_spec.rb +++ b/spec/bundler/bundler/fetcher/downloader_spec.rb @@ -98,6 +98,16 @@ end end + context "when the request response is a Net::HTTPForbidden" do + let(:http_response) { Net::HTTPForbidden.new("1.1", 403, "Forbidden") } + let(:uri) { Bundler::URI("http://user:password@www.uri-to-fetch.com") } + + it "should raise a Bundler::Fetcher::AuthenticationForbiddenError with the uri host" do + expect { subject.fetch(uri, options, counter) }.to raise_error(Bundler::Fetcher::AuthenticationForbiddenError, + /Access token could not be authenticated for www.uri-to-fetch.com/) + end + end + context "when the request response is a Net::HTTPNotFound" do let(:http_response) { Net::HTTPNotFound.new("1.1", 404, "Not Found") } diff --git a/spec/bundler/bundler/fetcher/index_spec.rb b/spec/bundler/bundler/fetcher/index_spec.rb index f0db07583cb4ab..971b64ce8f5538 100644 --- a/spec/bundler/bundler/fetcher/index_spec.rb +++ b/spec/bundler/bundler/fetcher/index_spec.rb @@ -63,26 +63,9 @@ context "when a 403 response occurs" do let(:error_message) { "403" } - 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 + it "should raise a Bundler::Fetcher::AuthenticationForbiddenError" do + expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationForbiddenError, + %r{Access token could not be authenticated for http://remote-uri.org}) end end