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.

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

Generate correct redirect URL when using TRUSTED_HOST #91

Open
alexandersch opened this issue May 11, 2021 · 5 comments
Open

Generate correct redirect URL when using TRUSTED_HOST #91

alexandersch opened this issue May 11, 2021 · 5 comments
Labels
Technical Debt Impacts only code quality, no or just small impact on end developers and users
Milestone

Comments

@alexandersch
Copy link
Contributor

When having set the TRUSTED_PROXIES and TRUSTED_HOSTS, generated URLs will use the URL provided by the X-FORWARDED-HOST header when fetching a page through the API (e.g. in a `text_editor' content type this will replace the absolute URLs with the provided URL in the header).

A page that has been configured as an internal link will generate a redirect when fetching the page through API. This is all correct behaviour.

The problem is that the generated redirect URL is then also using the forwarded host header. I think this should be the Sulu URL and not the provided URL by the forwarded host header.

Consequence is that the frontend follows redirects and fetches the wrong (redirected) URL, thus no JSON content from Sulu.

Please let me know if more info or examples are needed.

@alexander-schranz
Copy link
Member

alexander-schranz commented May 11, 2021

@alexandersch Thx for creating the issue. I think here was something misscommunicated in the slack channel.

The frontend application or the server rendered node js application need todo the following in this case e.g.:

  1. When visiting http://example.org/old-page
  2. The node-js or frontend-application does now request http://sulu.example.org/old-page.json
  3. Sulu will now return based on x-forwarded-host return http://example.org/new-page.json. This could also be any other route or url (e.g. https://www.google.com) so it is in this case required for the frontend application to check the returned url and not blindly follow this redirect.
  4. The JS application need in this case read the Location and call the used router.js with the new url (http://example.org/new-page). This router should then update the browser url with http://example.org/new-page and load the data from http://sulu.example.org/new-page.json correctly or in case of a external url redirect to the target.

So at current state from the backend handling here is correctly, but we need really make aware what all is needed to implement a correct router in your application handling redirects from the sulu backend correctly.

As example the http://example.org/new-page.json could be redirect to https://www.google.com or any other external resource its really needed that the redirect is never followed and that the application decides which url need to be routed internal by calling the js router again or which routes really will need todo a real redirect using in js window.location.href or the node server needed to return a redirect.

I hope I could make the problem more understandable that a redirect follow is here the problem in the frontend application and not the route which responded in the Location header. The following issues I created for that here is something missing in the docs and the example: #86.

@alexander-schranz
Copy link
Member

@alexandersch we did also have some discussion here that we could create maybe a optional activateable response listener which will make if a page.json is requested creating a response object like:

{
    "type": "redirect",
    "redirectType": "external", // or internal
    "url": "http://google.com/..",
}

But need to invest here some more time.

@niklasnatter
Copy link
Contributor

niklasnatter commented May 12, 2021

Thanks for creating this issue!

It looks like it is quite hard to configure fetch to not follow redirects automatically but return the target url of a redirect instead (whatwg/fetch#763). Because of this, I think it would be nice to make this easier in this bundle and allow to return some kind of type: redirect response like described by @alexander-schranz.

I think this is somehow related to returning the data for INTERNAL_LINK and EXTERNAL LINK pages (#62). Maybe it would be useful to implement the redirect response consistent to the data that is returned for a INTERNAL_LINK page?

@alexandersch
Copy link
Contributor Author

Ah, too bad fetch does not support this...

+1 for a redirect JSON response consistent to internal link page. I think this would make it a lot easier to implement a redirect in frontend application.

@alexander-schranz alexander-schranz added the Technical Debt Impacts only code quality, no or just small impact on end developers and users label May 28, 2021
@alexander-schranz alexander-schranz added this to the 1.0 Release milestone Sep 26, 2022
@alexander-schranz
Copy link
Member

If I remember correctly the projects where we used server side rendering over a JS framework are now setting the X-Forwarded-* headers in the vhost or even in index.php to fix this problem. Which I think is the way to go if the request can not set this headers. I added it to the 1.0 milestone as I think this is important part of the documentation of the headless bundle. As the headless bundle will have common usecase that it is request under a different domain then it need to output the things. And that JavaScript frameworks seems not allow this kind of things we need to document here alternative like vhost or how to set it in the index.php.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Technical Debt Impacts only code quality, no or just small impact on end developers and users
Projects
None yet
Development

No branches or pull requests

3 participants