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.

Sign up for GitHub

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

Jump to bottom

Configuration of username for index without enabling index #12543

Open
1 task done
PeterJCLaw opened this issue Feb 27, 2024 · 5 comments · May be fixed by #12748
Open
1 task done

Configuration of username for index without enabling index #12543

PeterJCLaw opened this issue Feb 27, 2024 · 5 comments · May be fixed by #12748
Labels
S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature

Comments

@PeterJCLaw
Copy link

PeterJCLaw commented Feb 27, 2024 •
edited
Loading

What's the problem this feature will solve?

I'm using keyring (with Google's plugin and to access Google Artifact Registry) via subprocess, which unfortunately means that pip doesn't get back a username. Currently this also means that one of pip/keyring prompts for a username as part of pip install --extra-index-url https://private.pypi.example/simple/.

As noted in #11971, this setup currently requires that the username (oauth2accesstoken) be included in the --extra-index-url.

I would like to be able to configure what username should be used (ideally globally) for a given index, without making pip then also enabling that index globally.

I am aware that the username can be specified in the --extra-index-url, however that isn't fully compatible with other tools.

Describe the solution you'd like

It would be great if there was a way to configure what username should be used for a given index without also telling pip to use that index everywhere.

The discussion at #11823 (comment) around a default-key-ring-user option feels like it would work for this case, though I don't feel strongly about that particular approach. (If we did go that way I'd suggest default-keyring-user as the keyring package name is not hyphenated)

Alternative Solutions

I'm semi working around this by setting a username in --extra-index-url https://[email protected]/simple/, however this doesn't seem to be an expected approach and causes issues for other tools. Notably GitHub Dependabot's config for private PyPIs refuses urls which have a username in them (and in my case I actually need to use a different username for Dependabot anyway) and will thus add the non-username spelling to my requirements files. This workaround therefore only partially works.

Additional context

It looks like #11827 previously discussed this as part of a wider set of changes, though I'm not clear what the status of that is.

It's clear others are hitting similar issues with subprocess keyring which this feels like it would solve -- #11971 for example.

Code of Conduct
@PeterJCLaw PeterJCLaw added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Feb 27, 2024
@PeterJCLaw PeterJCLaw mentioned this issue Feb 29, 2024
Open
1 task
@webknjaz
Copy link
Member

webknjaz commented Mar 3, 2024 •
edited
Loading

Is using ~/.netrc able to solve your issue?

@PeterJCLaw
Copy link
Author

Is using ~/.netrc able to solve your issue?

I haven't found a spelling which lets this work (it was something I tried). I agree that this feels like it would be a good solution if it could work alongside keyring.

For clarity: while ~/.netrc could solve the username portion, storing the password there is not an option -- not least from a security perspective but also because the password in this case is a short-lived token from Google's auth library rather than a persistent value.

From what I can tell, pip is able to use either ~/.netrc or credentials from keyring. The currently is that in pip's keyring handling, if it detects there's no username, then it will prompt the user for that username. Adding the username to the ~/.netrc doesn't appear to work as the username doesn't become available to the logic which is going to query keyring.

Looking in the code:

  • in _get_new_credentials, even if both netrc and keyring are allowed there's no passing of information from one to the other -- the only operate independently
  • of the two calls to _get_new_credentials only the one from within handle_401 enables keyring and that callsite also explicitly disables netrc

Do you have any insight into why netrc and keyring are kept so separate (despite being right next to each other in the code)?

@webknjaz
Copy link
Member

webknjaz commented Mar 4, 2024

No idea.

jfly added a commit to jfly/pip that referenced this issue Jun 5, 2024
@jfly
Add support for pulling username from keyring subprocess
dab840a 
This fixes pypa#12543 (and probably
pypa#12661).

Somewhat confusingly, when using using `PIP_KEYRING_PROVIDER=import`,
pip was able to fetch both a username and a password from keyring, but
when using `PIP_KEYRING_PROVIDER=subprocess`, it needed the username.

I opted not to write a new test for this, as the existing
[test_prompt_for_keyring_if_needed](https://github.com/pypa/pip/blob/86b8b23c1eaaac5c420a28519daf91de830f1182/tests/functional/test_install_config.py#L392)
feels sufficient to me.

TODO/need feedback:
- I haven't implemented any "feature detection" to see if the `keyring`
  subprocess supports `--mode=creds`. This feature was added quite
  recently
  (jaraco/keyring@7830a64
  landed in keyring v25.2.0). It looks like there's an older `getcreds`
  subcommand which we could probably feature detect as well. I
  personally would inclined to keep things simple and only support the
  latest version of the `keyring` cli, but I will gladly implement
  whatever y'all think is best.
- Should I add a news entry about this?
jfly added a commit to jfly/pip that referenced this issue Jun 5, 2024
@jfly
Add support for pulling username from keyring subprocess provider
5703f04 
This fixes pypa#12543 (and probably
pypa#12661).

Somewhat confusingly, when using using `PIP_KEYRING_PROVIDER=import`,
pip was able to fetch both a username and a password from keyring, but
when using `PIP_KEYRING_PROVIDER=subprocess`, it needed the username.

I opted not to write a new test for this, as the existing
[test_prompt_for_keyring_if_needed](https://github.com/pypa/pip/blob/86b8b23c1eaaac5c420a28519daf91de830f1182/tests/functional/test_install_config.py#L392)
feels sufficient to me.

TODO/need feedback:
- I haven't implemented any "feature detection" to see if the `keyring`
  subprocess supports `--mode=creds`. This feature was added quite
  recently
  (jaraco/keyring@7830a64
  landed in keyring v25.2.0). It looks like there's an older `getcreds`
  subcommand which we could probably feature detect as well. I
  personally would inclined to keep things simple and only support the
  latest version of the `keyring` cli, but I will gladly implement
  whatever y'all think is best.
- Should I add a news entry about this?
jfly added a commit to jfly/pip that referenced this issue Jun 5, 2024
@jfly
Add support for pulling username from keyring subprocess provider
1bf71c8 
This fixes pypa#12543 (and probably
pypa#12661).

Somewhat confusingly, when using using `PIP_KEYRING_PROVIDER=import`,
pip was able to fetch both a username and a password from keyring, but
when using `PIP_KEYRING_PROVIDER=subprocess`, it needed the username.

I opted not to write a new test for this, as the existing
[test_prompt_for_keyring_if_needed](https://github.com/pypa/pip/blob/86b8b23c1eaaac5c420a28519daf91de830f1182/tests/functional/test_install_config.py#L392)
feels sufficient to me.

TODO/need feedback:
- I haven't implemented any "feature detection" to see if the `keyring`
  subprocess supports `--mode=creds`. This feature was added quite
  recently
  (jaraco/keyring@7830a64
  landed in keyring v25.2.0). It looks like there's an older `getcreds`
  subcommand which we could probably feature detect as well. I
  personally would inclined to keep things simple and only support the
  latest version of the `keyring` cli, but I will gladly implement
  whatever y'all think is best.
This was referenced Jun 5, 2024
Open
Open
@jfly
Copy link
Contributor

jfly commented Jun 5, 2024

IMO, adding more configuration options feels wrong to me, as keyring does have support for retrieving both a password and a username. @PeterJCLaw, would #12748 solve your issue?

@PeterJCLaw
Copy link
Author

Yup, #12748 definitely looks like it could help with this, thanks! Agree that's a neater solution overall too.

jfly added a commit to jfly/pip that referenced this issue Aug 21, 2024
@jfly
Add support for pulling username from keyring subprocess provider
59ad998 
This fixes pypa#12543 (and probably
pypa#12661).

Somewhat confusingly, when using using `PIP_KEYRING_PROVIDER=import`,
pip was able to fetch both a username and a password from keyring, but
when using `PIP_KEYRING_PROVIDER=subprocess`, it needed the username.

I had to rework the existing tests quite a bit to fit with this new
behavior, as it's now OK to ask for a username/password for an index
even if you don't have a username. This is why `KeyringSubprocessResult`
now subclasses `KeyringModuleV2`. While I was in here, I opted to remove
the "fixtures" values from `KeyringModuleV2` by introducing this new
`add_credential` contextmanager. IMO, they were hurting the readability
of our tests: you had to jump around quite a bit to see what the
contents of the keyring would be during our tests. It also forced all
our tests to play nicely with the same fixtures values, which IMO was an
unnecessary constraint.

Note: per the discussion
[here](pypa#12748 (comment)),
I've opted not to implement any "feature detection" to see if the
`keyring` subprocess supports `--mode=creds`. For the record, this
feature was added in
jaraco/keyring@7830a64,
which landed in keyring v25.2.0.
jfly added a commit to jfly/pip that referenced this issue Aug 21, 2024
@jfly
Add support for pulling username from keyring subprocess provider
6d90fcb 
This fixes pypa#12543.

Somewhat confusingly, when using using `PIP_KEYRING_PROVIDER=import`,
pip was able to fetch both a username and a password from keyring, but
when using `PIP_KEYRING_PROVIDER=subprocess`, it needed the username.

I had to rework the existing tests quite a bit to fit with this new
behavior, as it's now OK to ask for a username/password for an index
even if you don't have a username. This is why `KeyringSubprocessResult`
now subclasses `KeyringModuleV2`. While I was in here, I opted to remove
the "fixtures" values from `KeyringModuleV2` by introducing this new
`add_credential` contextmanager. IMO, they were hurting the readability
of our tests: you had to jump around quite a bit to see what the
contents of the keyring would be during our tests. It also forced all
our tests to play nicely with the same fixtures values, which IMO was an
unnecessary constraint.

Note: per the discussion
[here](pypa#12748 (comment)),
I've opted not to implement any "feature detection" to see if the
`keyring` subprocess supports `--mode=creds`. For the record, this
feature was added in
jaraco/keyring@7830a64,
which landed in keyring v25.2.0.
jfly added a commit to jfly/pip that referenced this issue Aug 21, 2024
@jfly
Add support for pulling username from keyring subprocess provider
5592897 
This fixes pypa#12543.

Somewhat confusingly, when using using `PIP_KEYRING_PROVIDER=import`,
pip was able to fetch both a username and a password from keyring, but
when using `PIP_KEYRING_PROVIDER=subprocess`, it needed the username.

I had to rework the existing tests quite a bit to fit with this new
behavior, as it's now OK to ask for a username/password for an index
even if you don't have a username. This is why `KeyringSubprocessResult`
now subclasses `KeyringModuleV2`. While I was in here, I opted to remove
the "fixtures" values from `KeyringModuleV2` by introducing this new
`add_credential` contextmanager. IMO, they were hurting the readability
of our tests: you had to jump around quite a bit to see what the
contents of the keyring would be during our tests. It also forced all
our tests to play nicely with the same fixtures values, which IMO was an
unnecessary constraint.

Note: per the discussion
[here](pypa#12748 (comment)),
I've opted not to implement any "feature detection" to see if the
`keyring` subprocess supports `--mode=creds`. For the record, this
feature was added in
jaraco/keyring@7830a64,
which landed in keyring v25.2.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for pulling username from keyring subprocess provider jfly/pip
3 participants
@jfly @PeterJCLaw @webknjaz