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

Fixes #10577 - Handle login csrf token expiry nicely #7832

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

tbrisker
Copy link
Member

To test-

  1. set session expiry in the settings to 1 minute
  2. open two tabs. wait for both tabs to expire and log you out.
  3. log in with the first login page that was rendered (the second one will change the session and cause the first to expire). Alternatively, try logging in from both of them at the same time to see there isn't a redirect loop due to already being logged in.

@theforeman-bot
Copy link
Member

Issues: #10577

Comment on lines -397 to -402
# Called from ActionController::RequestForgeryProtection, overrides
# nullify session which is the default behavior for unverified requests in Rails 3.
# On Rails 4 we can get rid of this and use the strategy ':exception'.
def handle_unverified_request
raise ::Foreman::Exception.new(N_("Invalid authenticity token"))
end
Copy link
Member Author

Choose a reason for hiding this comment

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

About time we got rid of this now that we are on rails 6 :)

@@ -473,15 +473,15 @@ def user_set?(field)

def notifications
content_tag :div, id: 'toast-notifications-container',
'data-notifications': toast_notifiations_data.to_json.html_safe do
'data-notifications': toast_notifications_data.to_json.html_safe do
Copy link
Member Author

Choose a reason for hiding this comment

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

This and the rest below are just typo fixed i noticed while working on this pr.

ShimShtein
ShimShtein previously approved these changes Jul 20, 2020
Copy link
Member

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

LGTM, let's wait for the tests to pass.

Ron-Lavi
Ron-Lavi previously approved these changes Jul 20, 2020
Copy link
Member

@Ron-Lavi Ron-Lavi left a comment

Choose a reason for hiding this comment

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

Thanks, works great 👍

@ShimShtein ShimShtein merged commit e0d14b3 into theforeman:develop Jul 20, 2020
@ShimShtein
Copy link
Member

Merged, thanks, @tbrisker!

pcreech pushed a commit to pcreech/foreman that referenced this pull request Sep 1, 2020
Fixes #10577 - Handle login csrf token expiry nicely (theforeman#7832)

(cherry picked from commit e0d14b3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants