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

Auth: Ignore saving certain transitions #134

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Conversation

jeffdaley
Copy link
Contributor

@jeffdaley jeffdaley commented Apr 12, 2023

Ignores post-auth redirectTargets to the root and Authenticate routes. Fixes a bug causing valid redirects to be overwritten.

When you try to access an authenticated route, Ember Simple Auth restores your previous token, which, if expired, will cause the session to invalidate. By default, this reloads the Ember app in an effort to clear data. On refresh, the application beforeModel receives a new transition, this time to the application root. Currently we let this transition target through, overwriting the valid target. Now we ignore it :----)

@jeffdaley jeffdaley marked this pull request as ready for review April 12, 2023 20:23
@jeffdaley jeffdaley requested a review from a team as a code owner April 12, 2023 20:23
@@ -112,7 +112,7 @@ export default class SessionService extends EmberSimpleAuthSessionService {
}
}

if (redirectTarget && redirectTarget !== "/authenticate") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this to the beforeModel check

Copy link
Collaborator

@jfreda jfreda left a comment

Choose a reason for hiding this comment

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

@jeffdaley jeffdaley merged commit 447fef6 into main Apr 12, 2023
@jeffdaley jeffdaley deleted the jeffdaley/auth-errors branch April 12, 2023 20:47
anuragprafulla pushed a commit to razorpay/hermes that referenced this pull request Jun 27, 2023
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