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

Login return to #805

Merged
merged 7 commits into from
Jul 2, 2024
Merged

Login return to #805

merged 7 commits into from
Jul 2, 2024

Conversation

ajkiessl
Copy link
Contributor

@ajkiessl ajkiessl commented Jul 2, 2024

I believe this ultimately will fix the issue we are seeing with approvers (knock on wood). We were not applying any judgement to the "return_to" urls we were storing for session change/login between user types. This allowed for urls like '/approver.json' to be stored. I actually adapted some code from Scholarsphere to add some discretion to what we store. I still do not know exactly why a .json request was being used to login. Perhaps when azure resets a session it just takes the last request? If a user left their browser open to our approvers page for a while, then went to reload, this might've been triggered and the .json request was used?

ajkiessl added 7 commits July 1, 2024 13:21
…sn't default to Author. Also, only link committee members to approvers when directing to approver index so the linking doesn't happen whne datatables are loading
… valid. Else return_to the root path for that user model.
@@ -10,7 +10,15 @@ class AdminController < ApplicationController

def set_session
if current_remote_user.nil?
session[:return_to] = request.url
# Its important that the return_to is NOT stored if:
Copy link
Contributor

Choose a reason for hiding this comment

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

Appreciate these comments! A nice gift to future us

@ajkiessl ajkiessl merged commit 765088e into main Jul 2, 2024
1 check passed
@ajkiessl ajkiessl deleted the login-return-to branch July 2, 2024 15:44
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