New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WHATWG handling of URLs that include username:password (credentials) #3220
Comments
Do you have an example? |
https://fetch.spec.whatwg.org/#dom-request
|
Also: Isnt sending credentials via the url not a security issue, because it can be potentially logged? |
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. |
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 |
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. |
@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 |
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 |
@Eckhardt-D a few notes:
|
@mcollina I see, the part of the spec I referenced was incorrect. I was using |
I would have actually said, that you just write a wrapper. |
I think @mikaelkaron was working on something similar. |
I think an interceptor would work very well for this. |
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. |
This would solve...
Although many Node HTTP clients are moving away from handling
https://username:[email protected]
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.
The text was updated successfully, but these errors were encountered: