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 for users getting redirected to the wrong path #279

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

kdeal
Copy link
Contributor

@kdeal kdeal commented Oct 6, 2023

Users were no longer getting redirected back to the correct page after logging in. This was broken because passport started clearing the session store for users after authenticating them for security reasons in newer versions. This means the session.oauth2return is always empty after login, so users are always redirected to the home page. The oauth flow does allow us to send a state variable that will be forwarded to the callback url. It is recommended to either make the state variable a random string and store the state elsewhere or sign the data that you put into it. It seemed easier to use a random string and store the state in app.locals, so I went that route. The app.locals is in memory, so if it gets restarted the state will be lost. This is already true of our session store, so it shouldn't be any worse then our current state.

I manually tested this locally and it seems to work as expected. This is based off this article.

@kdeal kdeal marked this pull request as ready for review October 6, 2023 19:28
@kdeal kdeal requested a review from ymilki October 6, 2023 19:28
frontend/webapp.js Outdated Show resolved Hide resolved
Users were no longer getting redirected back to the correct page after
logging in. This was broken because passport started clearing the
session store for users after authenticating them for security reasons
in newer versions. This means the session.oauth2return is always empty
after login, so users are always redirected to the home page. The oauth
flow does allow us to send a state variable that will be forwarded to
the callback url. It is recommended to either make the state variable a
random string and store the state elsewhere or sign the data that you
put into it. It seemed easier to use a random string and store the state
in app.locals, so I went that route. The app.locals is in memory, so if
it gets restarted the state will be lost. This is already true of our
session store, so it shouldn't be any worse then our current state.
@kdeal kdeal merged commit 0dbd90c into Yelp:master Oct 6, 2023
3 checks passed
@kdeal kdeal deleted the fix_for_wrong_page branch October 6, 2023 20:27
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.

2 participants