Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Friend produces incorrect absolute redirects when behind a proxy #4

Closed
jumarko opened this issue Apr 20, 2022 · 0 comments · Fixed by #5
Closed

Friend produces incorrect absolute redirects when behind a proxy #4

jumarko opened this issue Apr 20, 2022 · 0 comments · Fixed by #5

Comments

@jumarko
Copy link

jumarko commented Apr 20, 2022 •
edited

A long time ago, they changed friend from using relative URLs to absolute for redirects: cemerick@5b04323

The default :login-uri is /login, which gets used in the "Location" header when generating redirects. Browsers may accept this, but it is not valid according to the HTTP spec, which requires an absolute URI.

However, this broke redirects, in case there's a proxy or a load balancer in front of the app terminating SSL/TLS: cemerick#84

The argument that "it is not valid according to the HTTP spec" hasn't been true for a long time.
There's an updated HTTP spec that allows using relative URLs for redirects: https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2

I suggest changing the code so it uses relative URLs for redirects again. That way, it will work out of the box.

Other resources:
jumarko added a commit to jumarko/friend that referenced this issue Apr 21, 2022
@jumarko
Use relative URLs for redirects.
f1b9c0a 
This fixes clj-commons#4.
For long, the HTTP spec has allowed relative URLs in the Location header,
see https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2.

Absolute URLs have been introduced here: cemerick@5b04323
It was trying to fix the issue reported in 2013: cemerick#42
where they claimed that friend doesn't follow the HTTP spec.

However, this lead to all sorts of problems with the clojure app
running behind an SSL/TLS proxy,
e.g. cemerick#84.

To sum up: Original friend implementation got it right by using
relative URLs for redirects but it wasn't, at the time,
strictly following the HTTP spec.
However, the HTTP spec has since been updated and there's no more reason
to use absolute URLs - they are brittle and break apps.
This was referenced Apr 21, 2022
Merged
Closed
slipset pushed a commit that referenced this issue Apr 24, 2022
@jumarko
[Fix #4] Stop enforcing absolute URLs for redirects. (#5)
d2ec66b 
For long, the HTTP spec has allowed relative URLs in the Location header,
see https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2.

Absolute URLs have been introduced here: cemerick@5b04323
It was trying to fix the issue reported in 2013: cemerick#42
where they claimed that friend doesn't follow the HTTP spec.

However, this lead to all sorts of problems with the clojure app
running behind an SSL/TLS proxy,
e.g. cemerick#84.

To sum up: Original friend implementation got it right by using
relative URLs for redirects but it wasn't, at the time,
strictly following the HTTP spec.
However, the HTTP spec has since been updated and there's no more reason
to use absolute URLs - they are brittle and break apps.

Use relative URL in default-unauthenticated-handler.

This is to avoid redirecting to HTTP when the user request was in fact to an HTTPS endpoint.
Such a situation happens when you run an SSL proxy in front of your plain HTTP app server.
In that case, `ring.util.request/request-url` returns _almost_ a proper endpoint,
but uses "http" not "https".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Get rid of enforcing absolute URLs for redirects. jumarko/friend
1 participant
@jumarko