Skip to content

P20-1722: Ensure clean session for assumed identity #70163

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.

Sign up for GitHub

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

Jump to bottom
Merged
artem-vavilov merged 1 commit into from
Jan 5, 2026
Merged

P20-1722: Ensure clean session for assumed identity #70163

artem-vavilov merged 1 commit into from
Jan 5, 2026

Conversation

@artem-vavilov
Copy link
Member

@artem-vavilov artem-vavilov commented Jan 1, 2026 •
edited
Loading

An attempt to fix the identity assumption issue, where the Account Settings page frequently shows the admin account instead of the assumed identity

Links

artem-vavilov
artem-vavilov commented Jan 1, 2026
View reviewed changes
dashboard/app/controllers/admin_users_controller.rb
Comment on lines +58 to +60
# Ensures a clean new session for the assumed identity
sign_out admin
reset_session
Copy link
Member Author

@artem-vavilov artem-vavilov Jan 1, 2026

Choose a reason for hiding this comment

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

code-dot-org/dashboard/app/controllers/sessions_controller.rb

Lines 75 to 81 in 0043b28

# GET /reset_session
def reset
client_state.reset
sign_out if current_user
reset_session
render layout: false
end

@artem-vavilov artem-vavilov requested a review from a team January 5, 2026 08:28
carl-codeorg
carl-codeorg approved these changes Jan 5, 2026
View reviewed changes
Copy link
Contributor

@carl-codeorg carl-codeorg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. I tested this locally and the admin functionality still works, although since I've never been able to repro the actual bug, I can't tell for sure if this will fix it. I think we should merge it and see if reports of the issue die down.

@artem-vavilov artem-vavilov merged commit 9b3f359 into staging Jan 5, 2026
6 checks passed
@artem-vavilov artem-vavilov deleted the P20-1722/assume-identity-fix branch January 5, 2026 18:48
@artem-vavilov
Copy link
Member Author

artem-vavilov commented Jan 5, 2026

LGTM. I tested this locally and the admin functionality still works, although since I've never been able to repro the actual bug, I can't tell for sure if this will fix it. I think we should merge it and see if reports of the issue die down.

I could not reproduce this either, but it looks like a session store issue, possibly a race condition or session ID caching that is not being reset correctly. Hopefully, manually resetting the previous (admin) session would fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carl-codeorg carl-codeorg carl-codeorg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@artem-vavilov @carl-codeorg