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

Update distributing guide to prefer twine's keyring support, and discourage use of pypirc #297

Open
theacodes opened this issue Apr 19, 2017 · 37 comments

Comments

@theacodes
Copy link
Member

@theacodes theacodes commented Apr 19, 2017

This can't be done until this upstream issue is resolved: pypa/twine#216
Original context: #271 (comment)

@glyph
Copy link

@glyph glyph commented Oct 23, 2017

I think this might actually be doable now. twine can't put a password in the keyring, but it can read one.

@theacodes
Copy link
Member Author

@theacodes theacodes commented Oct 23, 2017

@glyph what would it take for twine to be able to put a password in the keyring?

@ncoghlan
Copy link
Member

@ncoghlan ncoghlan commented Oct 24, 2017

@takluyver may know, as I believe flit already does this (at least on Linux).

@takluyver
Copy link
Member

@takluyver takluyver commented Oct 24, 2017

Yup, flit uses keyring on all platforms, so long as it's available (it's a soft dependency).

My strategy is to first try various means to get a stored or provided password. If none is found, we prompt the user, and then if keyring is importable, we store the password. You can see the code to do it here:

https://github.com/takluyver/flit/blob/8e1077c4d425239fea860b143714a2622c4d16db/flit/upload.py#L138-L168

The main UI weakness I'm aware of is that flit provides no mechanism to change the stored password; if you type it wrong, or change it later, you have to go through whatever mechanism the platform provides to change or remove it.

I hope that twine is storing the passwords in a similar way so that both tools can use one stored password.

@theacodes theacodes self-assigned this Dec 13, 2017
@theacodes theacodes changed the title Update distributing guide to mention twine configuration for storing passwords in the keyring Update distributing guide to prefer twine's keyring support, and discourage use of pypirc Dec 13, 2017
@theacodes
Copy link
Member Author

@theacodes theacodes commented Dec 13, 2017

@ncoghlan after discussing with Sumana on IRC about the security implications of .pypirc, I strongly think that the tutorials should discourage the use of .pypirc altogether and we should work towards our tools having keyring support. Let me know if you have any thoughts/reservations.

@dstufft
Copy link
Member

@dstufft dstufft commented Dec 13, 2017

I think something like twine configure would be awesome.

@ncoghlan
Copy link
Member

@ncoghlan ncoghlan commented Dec 14, 2017

@mbacchi
Copy link

@mbacchi mbacchi commented Jan 3, 2018

Here's a blog post I did on how to prevent .pypirc from being leaked for reference: http://mbacchi.github.io/2017/12/22/3-ways-prevent-secret-leaks-github.html

FYI @brainwane

@theacodes
Copy link
Member Author

@theacodes theacodes commented Jan 3, 2018

Thank you, @mbacchi! very helpful.

@brainwane
Copy link
Member

@brainwane brainwane commented Feb 1, 2018

I'm on Debian 9.3 (stretch, e.g., stable) and today I successfully made a username/password/website credential available for python-keyring to retrieve (so twine can use it when uploading a package).

I followed, basically, the instructions for Ubuntu 16.04 plus @jaraco's explanation in pypa/twine#208 (comment) , and everything worked out. Starting in a Python 3 virtualenv, and eliding a lot of output and a test python session where I tested that I could set and get a credential, here's what I did:

$ sudo apt install python3-venv libdbus-glib-1-dev
$ pip install secretstorage dbus-python
$ pip install keyring
$ keyring set https://test.pypi.org/legacy/ brainwane
Password for 'brainwane' in 'https://test.pypi.org/legacy/': # [TYPE THIS IN. NOT ECHOED.]
$ python setup.py sdist
$ python setup.py bdist_wheel
$ twine upload --repository-url https://test.pypi.org/legacy/ dist/*
Uploading distributions to https://test.pypi.org/legacy/
Enter your username: brainwane
Uploading Forms990_analysis-1.0.0-py3-none-any.whl
Uploading Forms990_analysis-1.0.1-py3-none-any.whl
Uploading Forms990-analysis-1.0.0.tar.gz
Uploading Forms990-analysis-1.0.1.tar.gz

My opinion: we should put a variant of this into the uploading tutorial and the guide to using Test PyPI, with adjustments for the user's OS.

@di
Copy link
Member

@di di commented Feb 1, 2018

I want to reiterate a comment I made here:

But arguably this isn't worth the effort, since the default access control granted by keyring is that any other Python application may access it (via keyring.get_password) and there doesn't seem to be a way to scope this just to twine (but perhaps I'm missing something).

To be more explicit, the following lines (run anywhere, not just from within twine) would give @brainwane back the password in plaintext:

>>> import keyring
>>> keyring.get_password('https://test.pypi.org/legacy/', 'brainwane')

I don't really see the value in putting this in the guide unless it's at least marginally more secure that keeping a password in ~/.pypirc, which it seems like it's not (but again, totally possible that I'm missing something here).

EDIT: Perhaps this is all understood, and it could be argued that using the keyring over ~/.pypirc helps prevents the leaking of passwords, so maybe it is worth doing after all. Just wanted to raise that point. 🙂

@takluyver
Copy link
Member

@takluyver takluyver commented Feb 1, 2018

It doesn't protect against untrusted code running in your user account, but storing the password in the keyring is an improvement in other ways:

  • If your computer is stolen, the thief can't access the keyring data unless they know your login password - the keyring is encrypted even if you haven't set up an encrypted filesystem (AIUI).
  • Your .pypirc file might be readable by other users who can log on to the same computer, if the permissions are not set correctly. The guide currently describes creating this file with no mention of setting its permissions, and at least on Linux the default file permissions allow anyone to read it.

There's arguably also a psychological reason: putting PyPI passwords in a plaintext config file signals that they're not that important. Using a specific mechanism for storing secrets would suggest that PyPA thinks how you store your password matters.

We definitely shouldn't see using keyring as the end of the road on this. Glyph has done some work on getting the credentials from a password manager, for instance. And hopefully Warehouse will add features like tokens to upload a specific project. But if we can recommend keyring rather than storing passwords in .pypirc, I think that's a step forwards.

@di
Copy link
Member

@di di commented Feb 1, 2018

All great points @takluyver. Consider me convinced, and thanks for laying out an explanation.

@brainwane
Copy link
Member

@brainwane brainwane commented Mar 8, 2018

We'll be publicizing pypi.org widely probably within the next two weeks, and I would very much welcome help on this, so the packaging guide (which we link to consistently within https://pypi.org/help/ and elsewhere on the site) mentions keying as a preferred credential store.

I've just put out a new release of twine and would be happy to put out a point release that includes command-line help with keyring plus guidance in the README (per pypa/twine#277). Then we could update the packaging and distribution guide, remove or move the .pypirc guidance, and point to the twine/keyring documentation.

@brainwane
Copy link
Member

@brainwane brainwane commented Mar 15, 2018

I'm working towards getting Twine 1.10.1 out in the next few days and would strongly welcome doc improvements related to keyring to get into that release!

@brainwane
Copy link
Member

@brainwane brainwane commented Mar 20, 2018

Twine 1.11.0 is now out and includes guidance in the README for using keyring.

@jonparrott and I have, in our experience, run into hiccups in getting keyring support to work well on Linux. We'll file bugs appropriately. Once the user experience is smoother, then the packaging/distro guide should replace .pypirc recommendations with keyring recommendations.

@jakethedev
Copy link

@jakethedev jakethedev commented Jun 29, 2018

Hey all, I'm a bit new to python packaging and have a general question about this particular bit of the docs, no idea about the best way to ask but this issue seems to be the most relevant place

The docs mention that one should install twine and run twine upload --repository-url https://test.pypi.org/legacy/ dist/* - but setup has this functionality built in, doesn't it? What's the difference between the twine command and python setup.py bdist_wheel (or any other preferred versions) upload -r https://test.pypi.org/legacy/? Is it mostly that twine has better authentication tooling?

I was thinking I'd pop over here and make a PR to add the setup-upload info to the docs and found this thread, so I hope it doesn't hurt to ask

@ncoghlan
Copy link
Member

@ncoghlan ncoghlan commented Jun 30, 2018

@jaktonik Depending on exactly which Python environment you're using, the native distutils upload may happen over HTTP, exposing your PyPI password as cleartext. There's also some newer metadata fields which distutils on its own won't extract and publish correctly, but twine makes sure are handled.

We should add that info as a footnote on https://packaging.python.org/guides/tool-recommendations/, perhaps by linking out to https://twine.readthedocs.io/en/latest/#why-should-i-use-this

@illume
Copy link
Contributor

@illume illume commented Aug 17, 2018

Hello wonderful people! 👋

I feel like the section in the python packaging guide about storing your password in plaintext in pypirc should be removed immediately. It's a separate issue of if twine keyring support is ready for everyone, or if alternative instructions are ready.

https://packaging.python.org/guides/using-testpypi/?highlight=username#setting-up-testpypi-in-pypirc

The note about setting permissions 'properly' is also not good advice. Instead it should just say Do not store passwords in the pypirc file. Storing passwords in plain text is never a good idea., and not have instructions on how to store plain text passwords in files.

The important part is removing the recommendation on storing passwords like this as soon as possible.

@illume
Copy link
Contributor

@illume illume commented Aug 17, 2018

Here's where the python docs talk about storing the password in pypirc: https://docs.python.org/3/distutils/packageindex.html#the-pypirc-file

@theacodes
Copy link
Member Author

@theacodes theacodes commented Aug 17, 2018

Hi @illume, feel free to see a pull request!

@illume
Copy link
Contributor

@illume illume commented Aug 17, 2018

Thanks @theacodes :)

@brainwane
Copy link
Member

@brainwane brainwane commented Oct 8, 2019

The Python docs now no longer mention .pypirc, yay! Thanks @kojoidrissa!

I'm having trouble figuring out when, but someone removed the .pypirc text from source/tutorials/packaging-projects.rst -- but it doesn't explicitly mention the keyring, so I think this issue is not yet resolved.

@pradyunsg
Copy link
Member

@pradyunsg pradyunsg commented Oct 8, 2019

@brainwane

I think the relevant file is source/guides/distributing-packages-using-setuptools.rst. It still mentions .pypirc and suggests creating a file with the user's password in plain text.

https://github.com/pypa/packaging.python.org/blob/c8339b381d0446b1c73aa94689869266beb0d334/source/guides/distributing-packages-using-setuptools.rst#create-an-account

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Jan 20, 2020

Checking in on the desired outcome of this issue. I've been passively tracking issues related to twine and credentials (most recently pypa/packaging-problems#313). My sense is that the documentation around .pypirc, API tokens, and keyring is fragmented and inconsistent. I searched this repo, warehouse, twine, and packaging-problems for mentions of those items; my findings are below.

Here is a list of possible next steps, which I hope prompts folks with opinions to chime in. :)

  1. Document the current commands necessary to use twine, keyring, and API tokens
    • Not sure if that belongs in this repo (as suggested by this issue), or twine (as suggested by pypa/twine#11)
  2. Document the format of .pypirc, including an example that relies on keyring and API tokens
    • Probably in this repo
  3. Update the docs in this repo, warehouse, and twine to be consistent with and reference the new documentation
  4. Streamline the process of configuring credentials for twine
    • See pypa/twine#216, pypa/twine#324, and pypa/twine#561 for ideas
    • It's tempting to do this first, but I think documenting the current process will be more immediately useful to package maintainers, and might give insight on how to update twine

The entry point for help using API tokens seems to be https://pypi.org/help/#apitoken (source), or the token management page (source). Both suggest storing the token in .pypirc; the former links to https://packaging.python.org/guides/distributing-packages-using-setuptools/#create-an-account, which provides this warning:

Be aware that this stores your token in plaintext.

Compare that to https://packaging.python.org/guides/using-testpypi/#setting-up-testpypi-in-pypirc:

Do not store passwords in the pypirc file. Storing passwords in plain text is never a good idea.

Reading over this issue, it seems the preferred solution is to use keyring. However, keyring doesn't seem to be documented in this repo or in warehouse. It's introduced at https://twine.readthedocs.io/en/latest/#keyring-support. I haven't seen any mention of how to use keyring with API tokens (though I suspect it'd be to use __token__ for the username).

Furthermore, there doesn't seem to be centralized documentation of .pypirc. In pypa/twine#340, there was an attempt to add it to the twine docs, but that was closed in favor of adding it to this repo.

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Jan 20, 2020

Related: #628 and pypa/twine#496 re: project-scoped API tokens. The suggestion seems to be to configure "proxy" repositories for each project, with distinct tokens.

I haven't investigated if/how those are supported by twine when using keyring. Maybe @jaraco has a quick answer?

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Jan 26, 2020

I haven't investigated if/how those are supported by twine when using keyring.

I did some investigation, and it's not obvious how to use multiple project API tokens with twine and keyring: pypa/twine#565.

@jaraco
Copy link
Member

@jaraco jaraco commented Jan 26, 2020

The twine/keyring integration assumes one password per user and index. What keyring needs to support multiple credentials is some way to distinguish the tokens by project name. Perhaps instead of relying on __token__ for the username, it should use PyPI token for {project} and fallback to PyPI token (but still transmit __token__ during the upload). Other syntax is possible, like __token__/{project}.

Per-project credentials wasn't considered in the original design, so a new design is called for.

@takluyver
Copy link
Member

@takluyver takluyver commented Jan 26, 2020

For flit, I was planning to stuff the project name associated with a token into the 'username' for keyring, e.g. token:{project}. I was assuming this would not be visible to users in normal operation. It would be good if flit and twine can settle on a common convention for this.

I know the secret storage protocol offered by Gnome allows for arbitrary key-value metadata associated with a secret, which can be used to look it up. In principle, this would be a neat way of storing a token scope, but it would only work for keyring if there are similar mechanisms on all supported platforms.

@jaraco
Copy link
Member

@jaraco jaraco commented Jan 26, 2020

+1 to having a common scheme shared by all tools. Just please don't require the 'project' to be specified. For a person managing a single project or two, that's fine, but where many projects share a token, you want keyring to store that credential exactly once. Perhaps token[:designator] where the user can provide an optional designator for the token (or none at all for the sole or primary token).

@takluyver
Copy link
Member

@takluyver takluyver commented Jan 26, 2020

I might go for something like token:user:takowl for a user-scoped token and token:project:pynsist for a project-scoped one.

I guess the precise details depend on whether this is something you expect users to see or enter manually, or more of an implementation detail. I'm hoping there will one day be an API to create a token (pypa/warehouse#6396 ), so if there isn't already a suitable token in keyring, the user can be prompted for their password (& 2FA), and it obtains and stores a new project-scoped token (rather than storing the password).

@jaraco
Copy link
Member

@jaraco jaraco commented Jan 26, 2020

I think it's a mistake to assume a token is uniquely associated with either a user or a project. A user can create a token for any purpose. For example, consider the tokens configured on my account:

image

I've created two tokens with the "all projects" scope, and another which I created for a specific project, but I could imagine in the future PyPI might even offer a scope for an organization or other subset of projects. What I think is needed here is a separate designator that the user could use for any purpose to select a key other than the default. Perhaps the designator could be :user:takowl or :project:pynsist, but it's probably best to keep it general.

Also important, I'm not sure :project:name is a good designator. It suggests the token is associated with the project, and while it might be limited to a project, it's still associated with the user, so really it's like :user:takowl+:project:pynsist. I wouldn't suggest something like that. I only share it to illustrate the relationships this field is trying to capture.

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Jan 27, 2020

It's cool to see this discussion evolve. Thanks for diving in. Here are some thoughts from a relative newcomer to the packaging ecosystem.

I tend to agree with @jaraco's take. To me, it seems like the key for distinguishing a token is its "Name", rather than its "Scope". Furthermore, this already works in twine for "All projects" tokens:

username = __token__
password = pypi-***

With that in mind, I like the optional username suffix, such as:

username = __token__:pytest-xprocess
username = __token__:gitlab.com/python-devs

keeping in mind that the suffix doesn't need to equal the "Name" on PyPI; it's just a local identifier for distinguishing it to keyring.

There's a related discussion starting at pypa/twine#496 (comment) re: how to use project-scoped tokens in .pypirc, where @di proposed a new format.

@takluyver
Copy link
Member

@takluyver takluyver commented Jan 27, 2020

I understand that tokens are more complicated than my idea encompasses. But I'm thinking about how tools are practically going to use them: it needs to be reliable and efficient to find the token to use to upload a given project.

Keyring doesn't have any API to search for several credentials at once, and I assume it's not easy to add that given its cross-platform, extensible nature. So we have to do a small number of specific lookups - in my thinking, we'd check for a per-project token, and if it's not found, fall back to trying a per-user token.

It makes sense to allow putting the tokens in .pypirc for people who prefer that (e.g. if you're on a headless Linux server that doesn't have Secret storage running), but I don't want to require that tokens in keyring are mapped through a name in .pypirc - that just seems like an extra thing to go wrong.

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Feb 1, 2020

Hoping to keep the discussion going, and possibly for clarification, here's the CLI workflow that I'm imagining, keeping in mind that this could be streamlined with .pypirc.

Account token (works today):

$ keyring set https://upload.pypi.org/legacy/ __token__
Password for '__token__' in 'https://upload.pypi.org/legacy/': 

$ twine upload --username __token__ dist/*

Project token (proposed in pypa/twine#565):

$ keyring set https://upload.pypi.org/legacy/ __token__:pytest-xprocess
Password for '__token__:pytest-xprocess' in 'https://upload.pypi.org/legacy/': 

$ twine upload --username __token__:pytest-xprocess dist/*

Another idea from pypa/twine#565, combined with one from @di in pypa/twine#496 (comment), would be for Twine to use the repository name, instead of the URL, for the keyring lookup:

Account token:

$ keyring set pypi __token__
Password for '__token__' in 'pypi':

$ twine upload --username __token__ dist/*

Project token:

$ keyring set pypi:pytest-xprocess __token__
Password for '__token__' in 'pypi:pytest-xprocess':

$ twine upload --repository pypi:pytest-xprocess --username __token__ dist/*

However, that's a more substantial change to Twine.

@ncoghlan
Copy link
Member

@ncoghlan ncoghlan commented Feb 2, 2020

@takluyver's suggested approach seems reasonable to me: if the given username is exactly __token__ look for the project-specific token first, then fall back to the generic token.

If the given username is something else, even if it starts with __token__:, then just use it as-is.

@bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Feb 5, 2020

if the given username is exactly token look for the project-specific token first, then fall back to the generic token.

With this approach, I'm having trouble visualizing the commands that a package maintainer would use to set an account and/or project token in keyring, and then use one of those tokens to upload their package with twine. Could somebody provide an example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet