Skip to content

Commit

Permalink
Add response error message output to HTTP Status 401 Errors in FileDo…
Browse files Browse the repository at this point in the history
…wnloader (#15983)

* add missing test dependencies PyJWT and pluginbase

* add tests for unauthorized downloads due to missing or bad credentials

Also change the TestFileServer to distinguish between the two cases
so that we can test them by replacing the bad credentials error
message "Not authorized" with "Bad credentials".

* add response error message to 401 error in file downloader

The source download step attempts to load source_credentials.json when it is present.
When following the recommended steps of passing credentials to the source_credentials.json via
os.getenv('BACKUP_USER') etc., this can result in the json string "None" when
no authentication is provided.
This effectively gets translated to the curl call when being read:

    curl -i -u None:None https://artifactory.example.org/artifactory/source-backup/

This causes an error with status code 401, which currently does not propagate
the error message and is slightly misleading, as a source_credentials.json is
provided but the content is wrong:

    ERROR: conanfile.py (example/1.0): Error in source() method, line 17
            get(self, self.url, sha256=sha256, strip_root=True)
            ConanException: The source backup server 'https://artifactory.example.org/artifactory/source-backup/' needs authentication: . Please provide 'source_credentials.json'

With this fix, the error message is propagated in the same way others in the
same block, resulting in a more useful message, although the "please provide"
is still misleading.

    ERROR: conanfile.py (example/1.0): Error in source() method, line 17
            get(self, self.url, sha256=sha256, strip_root=True)
            ConanException: The source backup server 'https://artifactory.example.org/artifactory/source-backup/' needs authentication: {
  "errors" : [ {
    "status" : 401,
    "message" : "Bad credentials"
  } ]
}. Please provide 'source_credentials.json'
  • Loading branch information
shoeffner authored Mar 29, 2024
1 parent e5994f9 commit c412990
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 2 deletions.
2 changes: 1 addition & 1 deletion conans/client/downloaders/file_downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def _download_file(self, url, auth, headers, file_path, verify_ssl, try_resume=F
raise AuthenticationException(response_to_str(response))
raise ForbiddenException(response_to_str(response))
elif response.status_code == 401:
raise AuthenticationException()
raise AuthenticationException(response_to_str(response))
raise ConanException("Error %d downloading file %s" % (response.status_code, url))

def get_total_length():
Expand Down
2 changes: 2 additions & 0 deletions conans/requirements_dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ parameterized>=0.6.3
mock>=1.3.0, <1.4.0
WebTest>=2.0.18, <2.1.0
bottle
PyJWT
pluginbase
21 changes: 21 additions & 0 deletions conans/test/unittests/tools/files/test_downloads.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,27 @@ def test_download_forbidden(self, _manual):
download(conanfile, file_server.fake_url + "/forbidden", dest)
assert "403 Forbidden" in str(exc.value)

def test_download_unauthorized_no_credentials(self, _manual):
conanfile, file_server = _manual
dest = os.path.join(temp_folder(), "manual.html")
# Not authorized without credentials
with pytest.raises(AuthenticationException) as exc:
download(conanfile, file_server.fake_url + "/basic-auth/manual.html", dest)
assert "401 Unauthorized" in str(exc.value)
assert "Not authorized" in str(exc.value)

def test_download_unauthorized_literal_none_credentials(self, _manual):
conanfile, file_server = _manual
dest = os.path.join(temp_folder(), "manual.html")
# os.getenv('SOME_UNSET_VARIABLE') causes source_credentials.json to contain
# "None" as a string literal, causing auth to be ("None", "None") or with
# a "None" token.
auth = ("None", "None")
with pytest.raises(AuthenticationException) as exc:
download(conanfile, file_server.fake_url + "/basic-auth/manual.html", dest, auth=auth)
assert "401 Unauthorized" in str(exc.value)
assert "Bad credentials" in str(exc.value)

def test_download_authorized(self, _manual):
conanfile, file_server = _manual
dest = os.path.join(temp_folder(), "manual.html")
Expand Down
2 changes: 1 addition & 1 deletion conans/test/utils/file_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def get_user_passwd(file):
auth = bottle.request.auth
if auth is not None:
if auth != ("user", "password"):
return bottle.HTTPError(401, "Not authorized")
return bottle.HTTPError(401, "Bad credentials")
return bottle.static_file(file, store)

auth = bottle.request.headers.get("Authorization")
Expand Down

0 comments on commit c412990

Please sign in to comment.