Repository navigation
Fix switch user w/ remember me (broke in last release) - #1117
Merged
Merged
Conversation
Goal 1: Ensure that when SessionGuard calls logout that the session is completely cleaned up. To accomplish this, we hook into the Logout event fired by SessionGuard::logout. The listener `InvalidSessionOnLogout` does the session invalidation and CSRF token regeneration. This is a win because we call `Auth::logout` in several other places but don't do the session invalidation, which is a problem. **NOTE**: Because this nukes the session, any alert messages, flash data, will be removed unless they're pushed _after_ logout. Goal 2: React to Remember Me token integrity failures. Add a middleware which does the Guard::check, and then checks to see if we've identified the integrity failure and begun a logout. If so, set an appropriate alert and redirect to login. Auth::check happens all over the place, and SessionGuard isn't an appropriate place to redirect / set messages, so do it in middleware! Other work: - Refactor login controller - when extending a method, keep parameters named the same. - Refactor UserRememberTokenController - instead of redirecting to logout, just perform the logout and set appropriate message to display on login form - SessionGuard::cycleRememberToken - ensure recaller token is valid before deleting. - Add tests covering a variety of cases using remember me. psalm fixes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When I scoped all the queries for Remember Me tokens to the currently logged in user, I would up breaking the 'switch user' feature when using remember me tokens
When using switch user, session user is different to the owner of the remember me token, so we need to refer to the original user id.
So this fixes the issue and provides some follow up work
Goal 1: Ensure that when SessionGuard calls logout that the session is completely cleaned up.
To accomplish this, we hook into the Logout event fired by SessionGuard::logout. The listener
InvalidSessionOnLogoutdoes the session invalidation and CSRF token regeneration.This is a win because we call
Auth::logoutin several other places but don't do thesession invalidation, which is a problem.
NOTE: Because this nukes the session, any alert messages, flash data, will be removed
unless they're pushed after logout.
Goal 2: React to Remember Me token integrity failures.
Add a middleware which does the Guard::check, and then checks to see if we've
identified the integrity failure and begun a logout. If so, set an appropriate alert
and redirect to login.
Auth::check happens all over the place, and SessionGuard isn't an appropriate place
to redirect / set messages, so do it in middleware!
Other work: