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

Show more helpful messages for invalid passwords #815

Merged
merged 5 commits into from Oct 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/815.feature.rst
@@ -0,0 +1 @@
Show more helpful messages for invalid passwords.
14 changes: 14 additions & 0 deletions docs/index.rst
Expand Up @@ -88,6 +88,20 @@ Using Twine

4. Done!

.. _entering-credentials:

.. note::

Like many other command line tools, Twine does not show any characters when
you enter your password.

If you're using Windows and trying to paste your username, password, or
token in the Command Prompt or PowerShell, ``Ctrl-V`` and ``Shift+Insert``
won't work. Instead, you can use "Edit > Paste" from the window menu, or
enable "Use Ctrl+Shift+C/V as Copy/Paste" in "Properties". This is a
`known issue <https://bugs.python.org/issue37426>`_ with Python's
``getpass`` module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigmavirus24 I added this note, and updated the code to it. What do you think?

https://twine--815.org.readthedocs.build/en/815/#using-twine

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might move this up and indent it underneath item 2, but that's more of a nit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did that at first, but it felt distracting. I also think it works better as a direct link outside of the steps.

More documentation on using Twine to upload packages to PyPI is in
the `Python Packaging User Guide`_.

Expand Down
24 changes: 24 additions & 0 deletions tests/test_auth.py
@@ -1,3 +1,4 @@
import getpass
import logging

import pytest
Expand Down Expand Up @@ -202,3 +203,26 @@ def test_logs_config_values(config, caplog):
"username set from config file",
"password set from config file",
]


@pytest.mark.parametrize(
"password, warning",
[
("", "Your password is empty"),
("\x16", "Your password contains control characters"),
("entered\x16pw", "Your password contains control characters"),
],
)
def test_warns_for_empty_password(
password,
warning,
monkeypatch,
entered_username,
config,
caplog,
):
monkeypatch.setattr(getpass, "getpass", lambda prompt: password)

assert auth.Resolver(config, auth.CredentialInput()).password == password

assert caplog.messages[0].startswith(f" {warning}")
23 changes: 22 additions & 1 deletion twine/utils.py
Expand Up @@ -18,6 +18,7 @@
import logging
import os
import os.path
import unicodedata
from typing import Any, Callable, DefaultDict, Dict, Optional, Sequence, Union
from urllib.parse import urlparse
from urllib.parse import urlunparse
Expand Down Expand Up @@ -237,11 +238,31 @@ def get_userpass_value(
if cli_value is not None:
logger.info(f"{key} set by command options")
return cli_value

elif config.get(key) is not None:
logger.info(f"{key} set from config file")
return config[key]

elif prompt_strategy:
return prompt_strategy()
warning = ""
value = prompt_strategy()

if not value:
warning = f"Your {key} is empty"
elif any(unicodedata.category(c).startswith("C") for c in value):
# See https://www.unicode.org/reports/tr44/#General_Category_Values
# Most common case is "\x16" when pasting in Windows Command Prompt
warning = f"Your {key} contains control characters"

if warning:
logger.warning(f" {warning}. Did you enter it correctly?")
logger.warning(
" See https://twine.readthedocs.io/#entering-credentials "
"for more information."
)

return value
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigmavirus24 What do you think of something like this? I haven't tried it on Windows (yet), but here's what it looks like with an empty password:

Uploading distributions to https://test.pypi.org/legacy/
Enter your username: brian
Enter your password: 
  Your password is empty. Did you enter it correctly?
  See https://pypi.org/help/#invalid-auth for more information.
Uploading twine-3.4.2.dev12+g6694f57.d20210619-py3-none-any.whl
100%|██████████| 41.7k/41.7k [00:00<00:00, 153kB/s] 
For more details, retry the upload with the --verbose option.
HTTPError: 403 Forbidden from https://test.pypi.org/legacy/
Invalid or non-existent authentication information. See https://test.pypi.org/help/#invalid-auth for more information.

If this seems reasonable, I'm happy to add a new section to Twine's docs, to keep this independent of a specific repository.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


else:
return None

Expand Down