Allow defining empty username and password in .pypirc #426

marob · 2018-11-19T18:32:46Z

In the case of a private repository (Nexus in my case) without any authentication, I haven't found a way not to have a prompt asking for username and password.

This PR allows to have the following configuration:

username:
password:

Note that I haven't tested for side effects.

In the case of a private repository (Nexus in my case) without any authentication, I haven't found a way not to have a prompt asking for username and password. This PR allows to have the following configuration: ```ini username: password: ``` Note that I haven't tested for side effects.

codecov · 2018-11-19T18:34:53Z

Codecov Report

Merging #426 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #426   +/-   ##
=======================================
  Coverage   78.29%   78.29%           
=======================================
  Files          14       14           
  Lines         751      751           
  Branches      108      108           
=======================================
  Hits          588      588           
  Misses        130      130           
  Partials       33       33

Impacted Files	Coverage Δ
twine/utils.py	`83.68% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89a35dc...92602d8. Read the comment docs.

jaraco · 2019-02-06T22:09:23Z

This change breaks twine for me. I have no .pypirc. Following this change, I get an error when trying to upload:

Uploading treehouse-0.1.0-cp37-cp37m-macosx_10_9_x86_64.whl
HTTPError: 403 Client Error: Invalid or non-existent authentication information. for url: https://upload.pypi.org/legacy/

jaraco · 2019-02-06T22:14:40Z

Before this change, twine would resolve my username from the TWINE_USERNAME environment variable. Now I suspect it's being loaded as ''.

jaraco · 2019-02-07T21:26:03Z

Should I open a new ticket on this issue?

sigmavirus24 · 2019-02-07T21:56:19Z

Yes, please. It's disappointing that our CI didn't catch this

jaraco · 2019-02-09T16:20:18Z

@marob Thinking about the code prior to this change, if you had put username: unused and password: unused in the .pypirc file, would that satisfy your use-case?

marob · 2019-02-09T17:07:36Z

I'm not for using a special value to indicate an absence of value.

I don't understand how the changes introduced in this PR can have the side effect of not taking env vars into account.

On my understanding, the ArgumentParser parses arguments on the command line and defaults to env var if the argument is not present.
Son as the code first checks cli_value coming from the the ArgumentParser before testing the value from the configuration file (so before my modifications), this should not be related.

jaraco · 2019-02-09T17:10:59Z

twine/utils.py

@@ -57,7 +57,7 @@
 def get_config(path="~/.pypirc"):
    # even if the config file does not exist, set up the parser
    # variable to reduce the number of if/else statements
-    parser = configparser.RawConfigParser()
+    parser = configparser.RawConfigParser(allow_no_value=True)


Today I learned that allow_no_value=True allows for:

[pypi] username password

But with allow_no_value=False, username= and password= are still valid and still result in empty strings.

With allow_no_value=False, if the config file is of the form:

[pypi] username password

the username and password key won't be available in the parsed config.
Or am I mistaking?

In that case, the parser will raise an Exception, but naked username or password was not the prescribed syntax; rather username: (which I assume is equivalent to username=) was.

Well, now I remember why I had to add allow_no_value=True in order to allow empty value.

What's the difference between adding : (or =), and using naked keys?
Is it parsed differently?

Yes. Adding the separator causes the value to be parsed as the empty string. Not including the separator creates a ParseException unless allow_no_value=True in which case the value is None.

OK. The intent of my PR was to be able to provide a None value in the configuration file in order to be anonymous.
Without this PR, you wouldn't be able to provide such value, so the CLI would systematically ask for a login/password (which I didn't want).

jaraco · 2019-02-09T17:12:20Z

I don't understand how the changes introduced in this PR can have the side effect of not taking env vars into account.

You're right, and I've learned more in #450.

jaraco · 2019-02-09T17:19:51Z

twine/utils.py

@@ -195,7 +195,7 @@ def get_userpass_value(cli_value, config, key, prompt_strategy=None):
    """
    if cli_value is not None:
        return cli_value
-    elif config.get(key):
+    elif key in config:


Given that config always defaults with {'username': None, 'password': None}, I now believe this change unconditionally disables prompts.

If you don't add username nor password keys in your configuration file, the keys won't be available in the parsed config object, so it should continue to the prompt_strategy part.

That's where you're mistaken. Keys are set unconditionally to default to None.

What if I don't have a server-login section (which is my case):

[distutils] index-servers = internal [internal] repository: ****** username password

The defaults still get copied into every repository.

jaraco · 2019-02-09T17:21:08Z

if you had put username: unused and password: unused in the .pypirc file, would that satisfy your use-case?

I'm not for using a special value to indicate an absence of value.

I'll take that as a yes.

And I agree, using 'unused' to have special meaning would be undesirable. But if all you're seeking to do is supply a value that's supplied but ignored, '' is hardly better and is in fact worse because it doesn't convey its purpose and can be conflated with other special values like None.

…allowed, as added in #426. Note that the tests pass even without 'allow_no_value'.

…password was indicated. Ref #426 #450.

marob · 2019-02-09T17:33:21Z

I'll take that as a yes.

It was a no.

jaraco · 2019-02-09T17:34:37Z

In cad2ccf, I add a regression test that would have blocked this change... and in d1123b3, I merge that passing test with this change to illustrate the failure, seen here.

jaraco · 2019-02-09T17:37:48Z

It was a no.

Does that mean your private PyPI server is rejecting unused:unused as the credentials but accepts : as the credentials? Or maybe there's something in the requests that filters out double-blank authentication.

marob · 2019-02-09T17:40:38Z

I don't know if it accepts unused:unused (but I suspect it won't, except if I created a special user for it).

The goal is to provide no username nor password (no authentication) at all.

…e config should bypass the prompt. Ref #426.

jaraco · 2019-02-09T17:43:53Z

I don't know if it accepts unused:unused (but I suspect it won't, except if I created a special user for it).

I would not expect you to have to create a special user. My suspicion was that your server was not requiring authentication at all, so was ignoring the empty authentication that was being passed.

In 3c0ad12, I've added another test, a test capturing the intention that you're intending to express in this PR, prior to the PR, which should fail first and pass after merging with this commit.

…biguity. Ref #426.

…, suppressing prompts and keyring resolution. Ref #426. Fixes #450.

jaraco · 2019-02-09T18:06:42Z

Or maybe there's something in the requests that filters out double-blank authentication.

I've confirmed that requests does not suppress double-blanks for auth:

$ requests.get('http://httpbin.org/headers', auth=('', '')).json()                                                                                       
{'headers': {'Accept': '*/*',
  'Accept-Encoding': 'gzip, deflate',
  'Authorization': 'Basic Og==',
  'Connection': 'close',
  'Host': 'httpbin.org',
  'User-Agent': 'python-requests/2.21.0'}}

marob · 2019-02-09T18:10:16Z

The use case is not to provide empty string values for the username and password (which will indeed generate a : basic authentication), but to provide None values to prevent the CLI to ask for login/password.

jaraco · 2019-02-09T18:16:10Z

@marob: Can you confirm then that with username= and password= in your config and the latest code in #452 that your Nexus server rejects the request because of the auth?

Regardless of that outcome, I tend to agree - one shouldn't have to supply special values, even empty ones, to avoid prompts. Unfortunately, due to the current behavior and expectations, the None value is already used to mean "unspecified, so prompt (maybe keyring)". A more involved refactoring is going to be required to get the behavior you're seeking.

* Add test capturing expectation that an empty username or password is allowed, as added in #426. Note that the tests pass even without 'allow_no_value'. * Add test capturing expectation that the password prompt occurs if no password was indicated. Ref #426 #450. * Add test capturing new expectation that an empty password found in the config should bypass the prompt. Ref #426. * Remove 'allow_no_value' on config parser. It was unneeded and adds ambiguity. Ref #426. * Allow empty strings in config to satisfy username and password values, suppressing prompts and keyring resolution. Ref #426. Fixes #450.

marob · 2019-02-09T19:19:29Z

It does work with 95ecde2 and the following configuration:

[distutils]
index-servers =
    internal

[internal]
repository: ******
username:
password:

jaraco · 2019-02-09T19:52:27Z

That's great news. We'll acknowledge that it's a hack - that a better design might be had, but at least there's a suitable workaround in the meantime. Thanks @marob for the extra effort in making this product better.

sigmavirus24 merged commit f4505d6 into pypa:master Jan 12, 2019

marob deleted the patch-1 branch January 12, 2019 15:19

jaraco mentioned this pull request Feb 7, 2019

Twine no longer prompts or resolves from keyring #450

Closed

jaraco reviewed Feb 9, 2019

View reviewed changes

jaraco added a commit that referenced this pull request Feb 9, 2019

Add test capturing expectation that an empty username or password is …

78c46f0

…allowed, as added in #426. Note that the tests pass even without 'allow_no_value'.

jaraco added a commit that referenced this pull request Feb 9, 2019

Add test capturing expectation that the password prompt occurs if no …

cad2ccf

…password was indicated. Ref #426 #450.

jaraco added a commit that referenced this pull request Feb 9, 2019

Add test capturing new expectation that an empty password found in th…

3c0ad12

…e config should bypass the prompt. Ref #426.

jaraco added a commit that referenced this pull request Feb 9, 2019

Remove 'allow_no_value' on config parser. It was unneeded and adds am…

c690931

…biguity. Ref #426.

jaraco added a commit that referenced this pull request Feb 9, 2019

Allow empty strings in config to satisfy username and password values…

446e6cb

…, suppressing prompts and keyring resolution. Ref #426. Fixes #450.

jaraco mentioned this pull request Feb 9, 2019

Restore prompts while retaining support for suppressing prompts #452

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow defining empty username and password in .pypirc #426

Allow defining empty username and password in .pypirc #426

marob commented Nov 19, 2018

codecov bot commented Nov 19, 2018 •

edited

Loading

jaraco commented Feb 6, 2019

jaraco commented Feb 6, 2019

jaraco commented Feb 7, 2019

sigmavirus24 commented Feb 7, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco Feb 9, 2019

marob Feb 9, 2019

jaraco Feb 9, 2019

marob Feb 9, 2019

jaraco Feb 9, 2019

marob Feb 9, 2019

jaraco commented Feb 9, 2019

jaraco Feb 9, 2019

marob Feb 9, 2019

jaraco Feb 9, 2019 •

edited

Loading

marob Feb 9, 2019

jaraco Feb 9, 2019

jaraco commented Feb 9, 2019 •

edited

Loading

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

Allow defining empty username and password in .pypirc #426

Allow defining empty username and password in .pypirc #426

Conversation

marob commented Nov 19, 2018

codecov bot commented Nov 19, 2018 • edited Loading

Codecov Report

jaraco commented Feb 6, 2019

jaraco commented Feb 6, 2019

jaraco commented Feb 7, 2019

sigmavirus24 commented Feb 7, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaraco commented Feb 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaraco Feb 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaraco commented Feb 9, 2019 • edited Loading

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

marob commented Feb 9, 2019

jaraco commented Feb 9, 2019

codecov bot commented Nov 19, 2018 •

edited

Loading

jaraco Feb 9, 2019 •

edited

Loading

jaraco commented Feb 9, 2019 •

edited

Loading