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

(fix): Enable Volto Login to Redirect Back to Original Route After Authentication #6419

Merged
merged 20 commits into from
Oct 24, 2024

Conversation

dobri1408
Copy link
Contributor

@dobri1408 dobri1408 commented Oct 20, 2024

There are situations where you may want the Volto login to appear on a route other than /login, as demonstrated in this example: https://github.com/collective/volto-authomatic/blob/a78f354e2c8fc4a41cf9d71d76a93f1d1627d268/src/index.js#L18. This implies that the Volto login should be accessible from any route invoked, thus the redirect after you are logged in should work also.

To achieve this, I removed the hardcoded /login route and implemented a regex to strip the last segment of the route where the login was initiated.

Copy link

netlify bot commented Oct 20, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit f9f6f61
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/671ad44d1cc91d0008d91b6d

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this change work in multi-language sites, too, where the route may have a language prefix?

packages/volto/news/6419.bugfix Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Collaborator

I think this is not a "fix" but a "feature". login was not broken. It could also be "breaking".

I have wanted this feature in Volto for a long time. Thanks for bringing it forward.

@dobri1408
Copy link
Contributor Author

@stevepiercy, yes it will work also on a multilingual site, because the login route is still the last one

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

News item looks good. This still needs technical review before merging.

@nileshgulia1
Copy link
Member

@dobri1408 Just thinking out loud, What about the cases where we have http://localhost:3000/login/page-one/page-two? (However its hard to imagine feasibility of these cases). As per current scenario we only replace paths ending with /login, given that we have Login component at that path. For custom paths its ideal to deal with it in its own component? 🤔

@JeffersonBledsoe
Copy link
Member

Perhaps I'm not understanding what this PR achieves, but can this not already be done by appending /login to any path? e.g. /my/long/path/login will return you to /my/long/path after you login.

This is works by the path being caught in routes.js and the props.location in Login.jsx that gets set by the router (from the wildcard catch in routes.js) is return_url which gets returned to after a successful login.

@stevepiercy
Copy link
Collaborator

Perhaps I'm not understanding what this PR achieves, but can this not already be done by appending /login to any path? e.g. /my/long/path/login will return you to /my/long/path after you login.

Confirmed!

https://demo.plone.org/vertical-spacing/grid-and-text/login

We should document this feature, as I never knew it existed. I imagine both end users and developers would want to know about it.

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeffersonBledsoe @stevepiercy The release note was a bit misleading, so I suggested an edit. This is not about solving the general problem of taking you to where you were going after an interstitial login page. It's just a fix for a specific case where the login form was served at /fallback_login instead of /login, and didn't redirect anywhere after logging in.

packages/volto/news/6419.feature Outdated Show resolved Hide resolved
packages/volto/news/6419.feature Outdated Show resolved Hide resolved
@davisagli davisagli merged commit 4b690be into main Oct 24, 2024
63 of 68 checks passed
@davisagli davisagli deleted the returnUrl-login branch October 24, 2024 23:12
sneridagh added a commit that referenced this pull request Oct 25, 2024
* main:
  Client transforms for reducer data (#6422)
  (fix): Enable Volto Login to Redirect Back to Original Route After Authentication  (#6419)
  Test with plone.restapi 9.8.0 (#6430)
  Add German translations for URL management control panel (#6429)
  URL Management control panel: add Edit feature (#6425)
  Remove SEAMLESS mention in bootstrap (#6424)
  Aliases control panel: Improve layout and add CSV upload (#6421)
  Fix site setup access at toolbar by using @actions endpoint. (#6413)
  Remove last CI tests still in 18 (#6418)
  Node 22 as LTS, main and recommended Volto version (#6371)
  Refactor Aliases control panel and support new filters (#6414)
  Remove the Plone app from apps folder (#6415)
  Release generate-volto 9.0.0-alpha.19
  Release @plone/providers 1.0.0-alpha.3
  Release @plone/components 2.0.0-alpha.16
  Release @plone/client 1.0.0-alpha.19
  Release @plone/types 1.0.0-alpha.21
  Some types improvements/fixes (#6412)
@dobri1408 dobri1408 mentioned this pull request Oct 30, 2024
sneridagh pushed a commit that referenced this pull request Oct 30, 2024
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

Successfully merging this pull request may close these issues.

5 participants