Ensure HTTP auth persists throughout session

[cquick01]

Trying to use HTTP-basic authentication with GitLab, but was running into an error

2023-11-03 22:20:57 [    INFO] proxpi._cache: Listing packages in index 'https://__token__:****@gitlab.domain.com/api/v4/groups/123/-/packages/pypi/simple'
2023-11-03 22:20:57 [   ERROR] proxpi: Exception on /index/my-package/ [GET]
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 2525, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1822, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1820, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1796, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
  File "/usr/local/lib/python3.10/site-packages/proxpi/server.py", line 172, in list_files
    files = cache.list_files(package_name)
  File "/usr/local/lib/python3.10/site-packages/proxpi/_cache.py", line 817, in list_files
    extra_files = cache.list_files(package_name)
  File "/usr/local/lib/python3.10/site-packages/proxpi/_cache.py", line 462, in list_files
    self._list_files(package_name)
  File "/usr/local/lib/python3.10/site-packages/proxpi/_cache.py", line 425, in _list_files
    response.raise_for_status()
  File "/usr/local/lib/python3.10/site-packages/requests/models.py", line 1021, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url: https://gitlab.domain.com/api/v4/groups/123/-/packages/pypi/simple/my-package

It seemed like the credentials were not being cached properly throughout the session. I dug around a bit, and adding this got things working for my private repo(s) as well as public PyPi.

[EpicWink]

Thanks for contributing.

• You'll need to make tests pass. It looks like replacing tuple[str, str] with typing.Tuple[str, str] will fix this (typing I think is aliased as t)
• Need to run linting (ie black)
• Needs comprehensive unit tests and integration tests. I'm happy to do this at some point if you can't figure it out
• You are overwrite the session auth with a basic auth (tuple), even if no credentials were provided. This is an issue when proxpi is used as a library and a session is provided with auth already configured

[cquick01]

Thanks for the feedback!

I was having trouble running the tests in CI and when running the tests locally they passed. Opening a PR in my own repo got the actions to run, which were now successful after my last push - https://github.com/cquick01/proxpi/actions/runs/6813566676/job/18528455278?pr=1

Does that satisfy the "comprehensive unit tests and integration tests" or are those separate?

I ran black as specified in the CONTRIBUTING.md file (python -m black src) after my last commit but no files changed.

6814c26 should make it so we only update the auth tuple if a username (or username+password) is provided in the url.

I believe the points mentioned should be fixed now. Please let me know if there is anything else to do, thanks!

[codecov-commenter]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (dc1db88) 89.78% compared to head (6814c26) 89.75%.
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   89.78%   89.75%   -0.03%     
==========================================
  Files           3        3              
  Lines         656      664       +8     
==========================================
+ Hits          589      596       +7     
- Misses         67       68       +1     

Files	Coverage Δ
src/proxpi/_cache.py	91.16% <87.50%> (-0.06%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[EpicWink]

Does that satisfy the "comprehensive unit tests and integration tests" or are those separate?

New functionality needs new tests. These tests should fail before adding the functionality, and succeed after.

This project currently only has integration tests, so don't worry about unit-tests

---

Two more things:

• This isn't the place to update the session: that should happen on initialisation
• This same session is used to talk to other indexes, and to download files. The auth is likely invalid for those servers. Instead, you probably want to use the auth parameter of session.get

[cquick01]

Hi, thanks again for the feedback and sorry for the delay in updating.

I updated the tests to support testing with credentials. Rather than creating new tests, added a new fixture that loops through some credentials to test with. This works together with the existing server, mock_root_index, and mock_extra_index fixtures. This is "mocked" authentication, however. Is it okay to do like this, or would you rather we add a new tests.utils.make_server_auth function that uses something like Flask-HTTPAuth?

Functionality-wise, it is working with both my private GitLab server as well as public PyPi packages.

Please let me know what you think!

[EpicWink]

Is it okay to do like this, or would you rather we add a new tests.utils.make_server_auth function that uses something like Flask-HTTPAuth?

No, what your doing is fine. It matches how users would use this library (with env-vars)

---

One thing is I don't know whether the server which files are downloaded from also needs that authentication. I suspect it works for GitLab because it returns pre-signed URLs in the file-list response, but other servers might return relative paths which need to go through the same server (especially important for sibling files, eg metadata).

I think this means we need to also pass the authentication to the download-file request