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

#638: Redirect to requested resource on login #665

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

dsuren1
Copy link
Collaborator

@dsuren1 dsuren1 commented Nov 13, 2023

Description

This PR handle redirection after login when login redirection was performed programatically (due to unauthorized access to a resource and not logged in). Previously requested resource is saved to localstorage and upon successful redirection (after login) the saved localstorage value is cleared

Issue

@dsuren1 dsuren1 added this to the MS4 VSR - Phase 2 milestone Nov 13, 2023
@dsuren1 dsuren1 self-assigned this Nov 13, 2023
@dsuren1 dsuren1 linked an issue Nov 13, 2023 that may be closed by this pull request
Copy link
Collaborator

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

I can not test it locally. Reading the code, I'm afraid this solution may create a little bug, if the user after redirection does not do the login
E.g.

  • I'm redirectred to login, so the variable is set in storage
  • Instead of doing the login (e.g. I do not have the credentials), I try (now or the day after) to access to mapstore visiting the home page and it will redirect me to the previous page, cancelling storage, that redirects me on login, re-setting the storage variable
  • and so on and so on ...

This may be make me do a wrong redirection (once or forever). Could you please test this use case?
If so, I can not actually figure out a solution, we have to think about it. We can limit somehow it with sessionStorage but the user will have anyway to close the browser/tab to reset it.

  • Is more secure to put this in sessionStorage instead of localStorage, so this behavior is limited to the current tab. I think it is in anycase the better place to put this information.

Copy link
Collaborator

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

Nice solution. Thank you.

@offtherailz offtherailz merged commit 7d63d83 into georchestra:master Nov 21, 2023
1 check passed
@offtherailz
Copy link
Collaborator

@ElenaGallo, could you please test this as well as it is deployed on our test instance ?
Thank you

@tdipisa
Copy link
Collaborator

tdipisa commented Nov 29, 2023

@dsuren1 test in DEV passed please backport to 2023.02.xx branch

dsuren1 added a commit to dsuren1/mapstore2-georchestra that referenced this pull request Nov 30, 2023
tdipisa added a commit that referenced this pull request Nov 30, 2023
[Backport 2023.02.xx] #638: Redirect to requested resource on login (#665)
@tdipisa
Copy link
Collaborator

tdipisa commented Dec 1, 2023

@catmorales @jusabatier this update has been also backported to 2023.02.xx. The docker image is also available for your test.

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

Successfully merging this pull request may close these issues.

Error with not logged redirection
3 participants