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

misc: make sure redirection is not lost after outside navigation #1712

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

ansmonjol
Copy link
Collaborator

@ansmonjol ansmonjol commented Sep 4, 2024

Fixes ISSUE-422

Context

If a user follows a link to Lago and is not redirected, they are asked to login.
Once logged in successfully, their reach their expected destination.

However, if a user logs in using a third party (like google SSO), this redirection logic breaks.
The reason is that during the thirdparty login process, they are redirected out the app and the "memory" of the destination page is lost.

Description

This PR addresses this situation and bring a fix using the browser's local storage.

When ever a user is

  • kicked out the app after following a link (needs to login)
  • Logs out the app (couldn't bypass this, this event looks just like a kick out)

they now "register" their last visited page (or destination in the first case) into their local storage.

This data is not lost when they use a thirdparty login (unless they use private navigation probably)

--

This is the best approach I found regarding those criteria

  • Speed of implementation
  • Number of files to change
  • Complexity

It's not ideal, not the best, but probably the fastest approach regarding our current implemented logic.

I opened a ticket to refactor this approach, but mostly to change our route event and history system.

@ansmonjol ansmonjol added the 🥷 chore This doesn't seem right label Sep 4, 2024
@ansmonjol ansmonjol self-assigned this Sep 4, 2024
Copy link
Collaborator

@keellyp keellyp left a comment

Choose a reason for hiding this comment

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

LGTM! As you said, not ideal but it's a nice workaround for the moment

@ansmonjol ansmonjol merged commit 862ff06 into main Sep 4, 2024
11 checks passed
@ansmonjol ansmonjol deleted the redirection-lost branch September 4, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🥷 chore This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants