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

Is password reset construct_url patch still needed? #159

Open
mauritsvanrees opened this issue Aug 8, 2024 · 1 comment
Open

Is password reset construct_url patch still needed? #159

mauritsvanrees opened this issue Aug 8, 2024 · 1 comment

Comments

@mauritsvanrees
Copy link
Member

I have setup a fresh Plone site with cookieplone and deployed it on a test server.

  • Volto 18.0.0-alpha.42
  • Plone 6.0.12

When I create a user, the password reset mail contains a link like
http://localhost:3000/passwordreset/4e972ab4b223413cbe3885ae6eec7d51?userid=maurits

I would expect the live domain instead of localhost. Yes, I tried this on the live server, not locally.

I found out there is a Volto Settings panel where you can set a frontend domain, and this is by default set to http://localhost:3000

If you fill in a / as value and submit, Volto sends this to the backend with the full domain url. Then a password reset works.
The alternative is to set en environment variable VOLTO_FRONTEND_DOMAIN in the backend, with the full domain including https.
This probably should be in documentation somewhere, but I could not find it.

The settings field is used in a construct_url patch in plone.volto. But when locally I go in with a pdb, I see that the frontend_domain that we get as navigation root url is already correctly http://localhost:3000.

The patch was presumably made to avoid getting http://localhost:8080/Plone as frontend domain. But Volto does a POST to http://localhost:3000/++api++/@users/maurits/reset-password, so the backend already sees localhost:3000 as domain, without needing the patch.

When you instead with curl do a POST to http://localhost:8080/Plone/++api++/@users/maurits/reset-password then the frontend domain is localhost:8080/Plone. So only with that url it would be needed.

So I think this patch is no longer needed with current Volto, probably because of the "seamless mode" introduced in Volto 14. Could that be true?

In my case I did an override to basically undo the patch, by registering my own mail_password_template with PasswordResetToolView and the original construct_url method, and the url in the email was fine, showing that the patch is not needed, at least not in my use case.

This trips up more people. I got contacted by someone a few months ago with this problem, but did not know where to start looking. If something was wrong, I would have expected to find problems in many more areas, not just a password reset url.
I am surprised I did not find more reports (maybe my search keywords were wrong), except an issue from 2021 which put me on the right path.

So I would be in favour of removing the patch.
Or we could keep the patch in case it is needed in other cases, but by default set frontend_domain to an empty string (and make it not required). Then the patch would be basically inactive, unless someone explicitly sets a frontend domain.

@davisagli
Copy link
Member

I expect the patch is still needed if the backend is called (by volto's proxy, or a frontend webserver) without a virtual hosting path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants