WIP: Fix - Cannot access private repository using token

[booleand]

This PR addresses a potential bug from 19.2 as discussed here.

Embedding a token in an index_url is not working.

For example:

pip install SomeProject --index-url="https://TOKEN@some.where/somerepo/"

throws an exception: pip._internal.exceptions.DistributionNotFound

Any guidance/feedback would be appreciated!

[chrahunt]

Resolves #6796.

[pradyunsg]

/cc @zooba since this was modified last in the keyring PR, which is what git bisect points to as well.

[booleand]

Potential test case

def test_get_single_credential():
    auth = MultiDomainBasicAuth()
    get = auth._get_url_and_credentials

    assert get("http://token@example.com/path") \
        == ('http://example.com/path', 'token', None)
    assert auth.passwords['example.com'] == ('token', None)

To place in:

pip/tests/unit/test_download.py

Line 493 in 2cd26a2

def test_get_credentials():

[zooba]

Converting None to "" sounds reasonable to me. Though there may also be issues with the caching if we parse "" out of a URL but are expecting/looking for None.

I've paged all of this code out of my head already, I'm afraid, so without going through and relearning it I can't really offer much help, sorry :)

[pradyunsg]

Hehe. No worries @zooba! Thanks for chiming in.

I'm hoping I'll be able to make time to look into this today or tomorrow.

[pradyunsg]

@booleand Would you mind if I pick this up from here?

[booleand]

@booleand Would you mind if I pick this up from here?

@pradyunsg OK

Just want to mention a few things:

1. Need to consider which conditional to use on this line as the behavior will be different:

pip/src/pip/_internal/download.py

Line 317 in f46b7bb

if username is None and password is None:

I suspect there may be edge cases which will result in unexpected behavior. In particular when multiple calls are being made to a MultiDomainBasicAuth object.

2. Here is a summary of changes in MultiDomainBasicAuth:

• _get_new_credentials now returns a username even if it doesn't locate a password.

• _get_url_and_credentials calls _get_new_credentials only when both username and password are empty. Though this may need to be reconsidered depending on requirements.

• _get_url_and_credentials stores username and/or password when either is found.

• __call__ now instantiates and calls a HTTPBasicAuth when either username or password is present. It also converts any NoneTypes to an empty string.

3. Here is a potential test case for MultiDomainBasicAuth.__call__ :

def test_basic_auth_with_url_embedded_token():
    with patch.object(pip._internal.download.HTTPBasicAuth, '__call__') as mock_auth:
        auth = MultiDomainBasicAuth()
        req = MockRequest("http://token@example.com/path")
        auth(req)
    assert mock_auth.called_with('token', '')

[pradyunsg]

Thanks @booleand!

consider which conditional to use on this line

I decided to preserve that conditional as is, and modified what _get_url_and_credentials would return. The guard, as it stands, seems to be correct to me.

summary of changes

Thank you -- these helped. :)

potential test case

I ended up augmenting existing tests: 0832006.