WHATWG handling of URLs that include username:password (credentials)

[Eckhardt-D]

This would solve...

Although many Node HTTP clients are moving away from handling https://username:password@domain.com URLs in the Request URL. Undici COULD follow WHATWG and avoid confusion as to why the URL works in e.g. window requests, but not http clients.

Currently any implementation built on top of Undici that needs to handle unknown input URLs, need to do the URL parsing and checking themselves for username and password values and setting it as a Base64 encoded Authorization header. Letting Undici handle this would remove the need to do that and still stay within spec.

The implementation should look like...

Follow the WHATWG Fetch Standard advice to automatically parse the credentials out of the URL and create the Authorization header. If Undici controls this on the lower level, less user error/performance mishaps is prone.

I have also considered...

That the username:password scheme is becoming less and less advocated for, but it is undoubtably still often being used. And people are sending requests out on these URLs at least a couple of times before realising how to make it work. If undici handles the URL parsing, it avoids sending credentials in unencrypted URLs (edit: unencrypted in e.g. loggers) at all.

[KhafraDev]

Do you have an example?

[KhafraDev]

and still stay within spec.

https://fetch.spec.whatwg.org/#dom-request

3. If parsedURL includes credentials, then throw a TypeError.

[Uzlopak]

Also: Isnt sending credentials via the url not a security issue, because it can be potentially logged? One more reason to let this behaviour die...

[Eckhardt-D]

and still stay within spec.

https://fetch.spec.whatwg.org/#dom-request

3. If parsedURL includes credentials, then throw a TypeError.

I would say that makes sense for the Request object itself. At the point where the Request is constructed, the parsed URL should not include the credentials anymore and throw if it does. But my suggestion is that the implementation of fetch / request does the parsing and removes the credentials and converts it to an Authorization header.

[Eckhardt-D]

Also: Isnt sending credentials via the url not a security issue, because it can be potentially logged? One more reason to let this behaviour die...

Yes, this is the concern of sending plain credentials in the URL. My suggestion is still NOT to dispatch the request with the original URL, but remove the auth part and convert it to a basic Authorization header before opening the connection.

I agree that this scheme should die, but it isn't dead and until it is HTTP libs have the responsibility to perform the URL cleansing / conversion, because users still have the ability / incentive to fetch these type of URLs.

For example:

Browsers still allow you to visit these URLs, and their security feature is that the username:password@ part of the URL scheme is not shown in the Address Bar once the request is made. (Not saying it's a good security feature).

[Eckhardt-D]

Do you have an example?

Recently in a small lib that I maintain: https://github.com/Eckhardt-D/mapsite I had a user complain that many of their customers that submit a site URL to crawl the sitemap of their 'hidden' page it errors. This was especially confusing to them since they could view the full sitemap in the browser using the same URL.

[Eckhardt-D]

@Uzlopak @KhafraDev I just found this previously closed issue - https://github.com/nodejs/undici/issues/913 so I assume this will not be planned. If none if the info I provided is convincing - this issue can be closed

[metcoder95]

At the core, I wouldn't advise adding it for the points previously mentioned; this might seem like a good thing to do but is not possible for undici to understand that the proposed behaviour is the one all users want and can lead to friction and unexpected problems.

You can compose an interceptor for that using Dispatcher.compose so you just do it once and have it across your implementations.

[mcollina]

@Eckhardt-D a few notes:

1. for fetch(), we should strive to do what the standard says. It seems this is covered by https://fetch.spec.whatwg.org/#dom-request, which states that it should throw. This contradicts the issue, which claims that we are not following the spec on this. Almost every single time we deviate from the standard there is a whack-a-mole of compatibility problems to fix, so it's better if we stay on that standard path as much as possible.
2. for undici.request() I think we should add some support for basic authentication at some point. I have an high suspicion that this could be implementable user-land via an interceptor.

[Eckhardt-D]

@mcollina I see, the part of the spec I referenced was incorrect. I was using undici.request() in my implementation and it did not throw, but got 400 responses. Would something like a BasicAuthAgent be viable?

[Uzlopak]

I would have actually said, that you just write a wrapper.

[mcollina]

I think @mikaelkaron was working on something similar.

[mcollina]

I think an interceptor would work very well for this.

[mikaelkaron]

I’m hacking on something similar, an interceptor for digest auth at https://github.com/mikaelkaron/undici-digest-interceptor.

so I agree with the above, interceptor is the way to go.