Clojurians Log v2
aleph
I noticed that Aleph’s HTTP client follows redirects by just replacing the URL and possibly the method. It will however keep all other request options, in particular any credentials from the original request.
That means that by default, if one sends credentials to an endpoint that responds with a redirect, Aleph will happily send these credentials along.
I’m not saying this behaviour is wrong, but I noticed that while debugging this scenario: GitHub’s Asset download REST endpoint requires authentication using a bearer token, and will respond with a 302 to a CDN URL. There seems to be some load balancing between different CDN providers and my redirect sometimes succeeded (AWS) and sometimes failed with 403 Unauthorized (Azure). Turned out that the Azure CDN choked on the access token from the original GitHub API request.
So from now on I will make sure to set :follow-redirects false whenever my request contains credentials. I got a bad feeling that this is not the default, so I am sharing it here to hear other people’s opinions 🙂
And of course in my scenario it is slightly annoying that I need to follow the redirect myself. Maybe it would be good to provide some form of hook to modify the redirect request before it is followed? It also did not help with debugging that I had no access to the failed request map when the redirect request threw an exception because of the 403.
According to https://datatracker.ietf.org/doc/html/rfc9110#section-15.4 you are right that we should remove Authorization (and also Cookie)
Quoting the relevant bit: > Consider removing header fields that were not automatically generated by the implementation (i.e., those present in the request because they were added by the calling context) where there are security implications; this includes but is not limited to Authorization and Cookie.
Well, okay, it doesn't say we must do this
There could of course also be valid reasons for wanting to preserve them
Yeah, true. I also think most client libs out there would forward all headers
So maybe default to the recommended behavior but provide an option for preserving?
That section mentions a whole bunch of other things to consider when following redirects which Aleph doesn't really do
https://github.com/clj-commons/aleph/blob/d039e84f360b6a81a13de7a4d6316807ef05d22e/src/aleph/http/client_middleware.clj#L297-L316 is the relevant function
Called by handle-redirects which does implement some of the method-related semantics
I'll file a ticket at least 🙂
I was looking at the source code as well 🙂 One could argue that Aleph is not a user agent by itself, so some of the RFC recommendations might not apply here: > Remove header fields that were automatically generated by the implementation A browser might add headers, but Aleph does not, right?
Indeed, I think none of those apply
Would be interesting to also look at what other clients are doing
https://github.com/clj-commons/aleph/issues/759#issuecomment-3074076708 😄
I had a quick look at Java’s HTTP Client (OpenJDK). Did not actually run it, but looked at the code. They seem to copy “user headers” as well.
heh bingo
Would you be so kind to also add that to the issue? 🙏
Had to update my comment about clj-http just now
but it still boils down to the same default behavior
I probably won't get around to working on this any time soon. Patches welcome, tho 🙂
I take it you can live with your workaround for the time being?
Yeah, sure! I can see if I can come up with a patch, but I cannot make promises right now 🙂
Thanks for being so responsive @dergutemoritz — really appreciate it!
Happy to provide feedback / rubber ducking
You're welcome 🙏
Thanks for choosing Aleph ™️ 😄