Auth provided in URL skipped if user field is blank

[shuckc]

Describe the bug

Some services publish a URL like:

https://:mypassword@ropsten.infura.io/v3/..."

You can see the user field is an empty length string. When a request is made using aiohttp, the credentials are ignored and the server responds with a 403. This differs from requests and also curl, which parse this URL as having a zero-length username and a valid password.

To Reproduce

The request is sent without credentials

Expected behavior

Credentials should be sent

Logs/tracebacks

N.A.

Python Version

$ python --version
Version: 3.7.4.post0

aiohttp Version

$ python -m pip show aiohttp

multidict Version

$ python -m pip show multidict
Version: 5.1.0

yarl Version

$ python -m pip show yarl
Version: 1.6.3
...

OS

macOS

Related component

Client

Additional context

In asyncio/helpers.py specifically invoking strip_auth_from_url calls BasicAuth.from_url line, which contains:

@classmethod
def from_url(cls, url: URL, *, encoding: str = "latin1") -> Optional["BasicAuth"]:
    """Create BasicAuth from url."""
    if not isinstance(url, URL):
        raise TypeError("url should be yarl.URL instance")
    if url.user is None:
        return None
    return cls(url.user, url.password or "", encoding=encoding)

The underlying cause appears to be that yarl returns url.user=None and url.password=mypassword. You can see that url.password is not checked in this case. I will open a PR with suggested fix and test.

Code of Conduct

• I agree to follow the aio-libs Code of Conduct

[webknjaz]

I know that cURL doesn't do post-processing of the user input and can send invalid queries if you ask it to so it's a not very good reference.

Also, it looks like this may be a corner case, RFC-wise:

The userinfo subcomponent may consist of a user name and, optionally,
scheme-specific information about how to gain authorization to access
the resource.
[...]
Use of the format "user:password" in the userinfo field is
deprecated. Applications should not render as clear text any data
after the first colon (":") character found within a userinfo
subcomponent unless the data after the colon is the empty string
(indicating no password). Applications may choose to ignore or
reject such data when it is received as part of a reference and
should reject the storage of such data in unencrypted form. The
passing of authentication information in clear text has proven to be
a security risk in almost every case where it has been used.

(RFC 3986, section 3.2.1)

Plus, there are mentions of deprecations in various browsers too: https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#access_using_credentials_in_the_url.

This basically means that the username is mandatory and the password is optional but not vice versa. I've seen some services (like GitHub) allowing the use of tokens in the username field to work around this.

So technically, I'd say that aiohttp is RFC-compliant. But maybe if there are other libs (that are not requests) allowing an invalid missing username, we could look into what use-cases they try to cover.

[shuckc]

Whilst working on a patch I noticed there seems to be two places the url.user is checked for basic auth - I've addressed both hopefully in PR #6495:

aiohttp/helpers.py:181
aiohttp/client_reqprep.py:303

[shuckc]

Responding to your comments, I don't think the use of credentials in URLs is going to go away anytime soon, deprecated by the RFC or not, and I'm not sure that discussion of deprecation of the wider feature is relevant to handling this edge case.

The rest of the quotation "Applications should not render as clear text any data..." to me at least seems to be talking about the "URL bar" in the web browser, and in mouseover on hyperlinks ie. we hide the password entered from the user so it would not show on screenshots etc. This seems a sensible precaution - but not really relevant here?

I think that for web browsers with a human operator, removing HTTP Basic Auth and auth in URLs is the way forward - they are used rarely and from a usability point of view the 'authentication failed' workflow is quite poor. It's better to move the auth token into the path or query parts, by sending a long one-time URL and deal with expiry within a successful HTTP web page response that the user will understand and trust.

Specifically dealing with a http client library driving an API, the use of credentials embedded into a URL is totally common place, as is the case here. We may as well support the edge cases and be compliant with both observed behavior in other libraries, and required behavior by some services.

Of course the developer can work around this by cracking the URL first, reassembling it without the authentication, and passing in our own explicit auth=BasicAuth() instance, but it's nicer when this works out the box.

[webknjaz]

The rest of the quotation "Applications should not render as clear text any data..." to me at least seems to be talking about the "URL bar" in the web browser, and in mouseover on hyperlinks ie. we hide the password entered from the user so it would not show on screenshots etc. This seems a sensible precaution - but not really relevant here?

I think that for web browsers with a human operator, removing HTTP Basic Auth and auth in URLs is the way forward - they are used rarely and from a usability point of view the 'authentication failed' workflow is quite poor. It's better to move the auth token into the path or query parts, by sending a long one-time URL and deal with expiry within a successful HTTP web page response that the user will understand and trust.

Specifically dealing with a http client library driving an API, the use of credentials embedded into a URL is totally common place, as is the case here. We may as well support the edge cases and be compliant with both observed behavior in other libraries, and required behavior by some services.

I must note that another side of this precaution concerns logging — anything included in the URL (authority or query-string) can potentially leak to the logs in multiple proxies (it's not uncommon that there's at least one, and each place needs to be sanitized). So no, it's not only a user-facing representation.

Of course the developer can work around this by cracking the URL first, reassembling it without the authentication, and passing in our own explicit auth=BasicAuth() instance, but it's nicer when this works out the box.

Agreed.