Repository navigation
Fix lockout from stale session[:current_role] when a different user signs in - #5690
Merged
Merged
Conversation
Users::SessionsController signs out an already-signed-in user with a scoped sign_out, which keeps the rest of the session intact, so session[:current_role] from the previous user survives into the new login. current_role then resolved to a role the new user does not hold, authorize_user failed on every page including the dashboard, and the user was stuck in a redirect loop until they cleared cookies. Scope the session role lookup to the current user's own roles, and drop a leftover session role that does not belong to the newly signed-in user. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AoUPX63nd1LSkKDPEiQ7jt
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.
Description
Signing in as one user and then signing in as a different user in the same browser (without signing out first) locks the second user out of the app entirely with an infinite redirect loop. The only escape is clearing cookies.
Cause:
Users::SessionsController#createrunssign_out_if_signed_in, which is a scopedsign_out— it clears the Warden user but keeps the rest of the session, sosession[:current_role]from the previous user survives into the new login.session[:current_role] ||= ...then sees a present value and leaves it alone.ApplicationController#current_roledid a globalRole.find_by(id: session[:current_role]), so it happily returned a role the newly signed-in user does not hold.current_organizationderived from that foreign role,authorize_userfailed for every page — including the dashboard it redirects to — and the browser looped untilERR_TOO_MANY_REDIRECTS.This was hit organically by two of us while doing local QA on another branch.
Fix, in two parts:
current_rolescopes the session-role lookup tocurrent_user.roles, so a role the user doesn't hold is ignored and it falls back toUsersRole.current_role_for. This alone stops the lockout.SessionsController#createdeletes a leftoversession[:current_role]that doesn't belong to the newly signed-in user, so the session doesn't keep carrying another user's role around.Type of change
Bug fix.
How Has This Been Tested?
Two new specs, both confirmed failing before the fix and passing after:
spec/requests/sessions_requests_spec.rb— signing in as a second user in the same session reaches a successful dashboard and stores that user's own role.spec/controllers/application_controller_spec.rb—current_roleuses a session role the user holds, and ignores one belonging to another user.Also verified manually in a browser against local seed data: sign in as
org_admin1@example.com, then sign in asorg_admin2@example.comwithout signing out. Onmainthe second login dead-ends in a redirect loop; with this change it lands on the SF Diaper Bank dashboard, and/dashboardcontinues to load correctly.Full
spec/requests/sessions_requests_spec.rb,spec/controllers/application_controller_spec.rb,spec/requests/users_requests_spec.rbandspec/requests/dashboard_requests_spec.rbpass (the existing session-role-preference cases around last-used and revoked roles still hold). Rubocop clean on the changed files.🤖 Generated with Claude Code
https://claude.ai/code/session_01AoUPX63nd1LSkKDPEiQ7jt