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 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions tests/test_auth.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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?")
# TODO: Link to new entry in Twine docs
logger.warning(
" See https://pypi.org/help/#invalid-auth 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