WHATWG handling of URLs that include username:password (credentials) about undici HOT 14 OPEN

@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.

from undici.

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.

from undici.

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.

from undici.

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

from undici.

I think an interceptor would work very well for this.

from undici.

Do you have an example?

from undici.

and still stay within spec.

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

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

from undici.

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...

from undici.

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.

from undici.

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).

from undici.

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.

from undici.

@Uzlopak @KhafraDev I just found this previously closed issue - #913 so I assume this will not be planned. If none if the info I provided is convincing - this issue can be closed

from undici.

@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?

from undici.

I think @mikaelkaron was working on something similar.

from undici.