Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Keyring being only used when 401 is returned #11721

Open
1 task done
MarianG opened this issue Jan 11, 2023 · 6 comments
Open
1 task done

Fix Keyring being only used when 401 is returned #11721

MarianG opened this issue Jan 11, 2023 · 6 comments
Labels
C: keyring Related to pip's keyring integration S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature

Comments

@MarianG
Copy link

MarianG commented Jan 11, 2023

What's the problem this feature will solve?

Currently we have the issue that our artifactory returns http-403 for the initial lookup and does not use keyring credentials in the following. We found that those credentials are only used if a http-401 is returned.

So it would be great if there is a choice for HTTP return codes or if also 403 - forbidden could be included in the workflow to retry the request with credentials afterwards.

Describe the solution you'd like

One way to achieve this is to change the network/auth.py file. The handle_401 method might be adjusted to include other HTTP-codes as well, e.g.

   
   def handle_401(self, resp: Response, **kwargs: Any) -> Response:
        # We only care about 401 and 403 responses, anything else we want to just
        #   pass through the actual response
        status_codes = [401, 403]
        if resp.status_code not in status_codes:
            return resp
            .....

Alternative Solutions

Other workarounds did not work so well, since the artifactory return codes are fixed. I found some issues which mention to force keyring, which will work, but it is not implemented right now.

Additional context

Code of Conduct

@MarianG MarianG added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Jan 11, 2023
@JGoldstone
Copy link

I am similarly being bitten by this in using Artifactory, and the above simple fix would make my life easier. Yes, one could take the view that it's a moral hazard to compensate for an arguably incorrect behavior on Artifactory's part, but ... leaving things as they are only encourages lax security practices such as passwords in cleartext protected only by the user's having [one hopes] remembered to set the file mode bits conservatively on pip.conf.

@uranusjr
Copy link
Member

Did anyone attempt to submit the issue to Artifactory?

@jomach
Copy link

jomach commented Mar 14, 2023

We have the same problem.

@akpircher
Copy link

@uranusjr I don't know that making the change in artifactory is necessarily the right move. The artifactory PyPi repository allows setting permissions by "namespace", so that some users can install a python package while others can't. (This can result in some rather... non-pythonic solutions for python package names.)

This also means that some namespaces are available for anonymous download. So an initial connection without a username or password is a valid authentication, it just means you won't be able to access the packages that are protected, causing artifactory to return a 404 (if the permissions make it invisible) or a 403 error.

It would be nice if we could tell pip that certain indexes always need to get authenticating information from keyring. The workaround that I've been using is embedding my username in the index url, but that's... not really the best solution, and a bit of a pain.

I would love to be able to provide repeatable, non-generalized, installation instructions in my package documentation that looks something like pip3 install --with-authentication -i ... which just tells pip "hey, I know this server needs authentication, please go get it for me"

The documentation mentions --force-keyring/PIP_FORCE_KEYRING which SOUNDS like it's supposed to force pip to get authentication from keyring the way I want (like a --with-authentication), but it doesn't seem implemented.

@akpircher
Copy link

Disclaimer:
I am not attempting to defend artifactory design decisions, it's just the system some of us are stuck with using.

If it turns out this is still be something that needs to be fixed on artifactory's end, I'll gladly go request they return a 401 on protected resources during anonymous authentication, but that solution doesn't really sound right.

@odormond
Copy link

If it turns out this is still be something that needs to be fixed on artifactory's end, I'll gladly go request they return a 401 on protected resources during anonymous authentication, but that solution doesn't really sound right.

They do return a 401. But the authentication is not anonymous, at least not when using an index-url of the form https://[email protected]. The request sent by pip will have an Authentication header with the basic credentials set to the base64 encoding of username: (empty password). Due to the presence of the username, they temporarily suspend the access of that particular user. On the first failure the temporary suspension is of 1 second but things becomes ugly when you retry too soon. It's still not clear to me how it behaves and I got the impression that multithreading is making things even muddier.

Anyway, this used to work with pip >= 19.2 <= 21.1 as the keyring was always queried first. So I ended up implementing the documented --force-keyring and have just opened a PR for it: #12473.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: keyring Related to pip's keyring integration S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

7 participants