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 sign in redirect when token is expired #1510

Merged
merged 3 commits into from
Mar 11, 2019
Merged

Fix sign in redirect when token is expired #1510

merged 3 commits into from
Mar 11, 2019

Conversation

Betree
Copy link
Member

@Betree Betree commented Mar 8, 2019

Fix opencollective/opencollective#1797

In #1434 I made some changes to properly redirect a user that would click on a signin link with an invalid token while already being logged in.

However I made a false assumption while working on it as I considered that an invalid token was the same things that an expired token.

As a consequence, the bug would still appear for a user that would click on an old sign in link.

Changes

  • On /signin/{token}?next=/, if token is expired, we still try to login with the localStorage token
  • Added a cypress helper to generate tokens on the client side
  • Added tests that use valid but expired tokens
  • Added a assertLoggedIn helper for tests

@vercel
Copy link

vercel bot commented Mar 8, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@znarf
Copy link
Member

znarf commented Mar 9, 2019

@Betree does that take care of the part 2) of the issue (broken redirect in second Sign In attempt?)

@Betree
Copy link
Member Author

Betree commented Mar 10, 2019

@znarf I suspect this issue was because we stored a corrupted token in localStorage (expired tokens were still stored there). So it should normally be fixed.

I've added a test to ensure redirect is properly handled regardless of how next url is encoded.

@Betree
Copy link
Member Author

Betree commented Mar 10, 2019

I've just modified editCollective page to use the new UserProvider to deal with a recurring bug with the getCollectiveToEditData being fired before user was logged in which resulted in random loading errors, particularly on CI.

@Betree Betree merged commit 97b5bdb into master Mar 11, 2019
@Betree Betree deleted the fix/signin branch March 11, 2019 09:15
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