search

pelargir / auto-session-timeout

Provides automatic session timeout in a Rails application.
MIT License
126 stars 63 forks source link

Add compatibility for Rails protect_from_forgery #15

Closed kayleiburke closed 5 years ago

kayleiburke commented 7 years ago

Make auto-session-timeout compatible with Rails' protect_from_forgery feature by adding a condition to prevent the session from being timed out if the user is logging in. This condition is necessary because the user will receive a 'CSRF token invalid' error if they try logging in after they have been on the login page for longer than the configured timeout value (i.e. the time specified by auto_session_timeout in application_controller.rb).

If a developer wishes to take advantage of this feature, they can simply add a parameter to the auto_session_timeout declaration in application_controller.rb. The parameter indicates the relative path of the sign in page. For instance: auto_session_timeout 1.hour, '/users/sign_in'

pelargir commented 7 years ago

Thanks for the PR. I like this change but appending a ternary to the end of line 10 makes that line way too long. Please clean that up a bit and I'll be happy to merge this.

My suggestion is to assign the result of the ternary to a descriptive variable after line 9 and use that variable in the conditional on line 10.

kayleiburke commented 7 years ago

Thank you for the suggestion! I will implement this and will do another pull request.

Kaylei

On Tue, May 23, 2017 at 4:02 PM, Matthew Bass notifications@github.com wrote:

Thanks for the PR. I like this change but appending a ternary to the end of line 10 makes that line way too long. Please clean that up a bit and I'll be happy to merge this.

My suggestion is to assign the result of the ternary to a descriptive variable after line 9 and use that variable in the conditional on line 10.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pelargir/auto-session-timeout/pull/15#issuecomment-303531232, or mute the thread https://github.com/notifications/unsubscribe-auth/AEZpRPdhGVcTbqhbumchGXWMCWpi3xc-ks5r80lcgaJpZM4NihYS .

pelargir commented 7 years ago

@kayleiburke are you going to make this change and submit another PR? I'd be happy to accept it if you did.

davegudge commented 5 years ago

@pelargir I've opened https://github.com/pelargir/auto-session-timeout/pull/21 with your recommendation above.

pelargir commented 5 years ago

@davegudge thanks. #21 has been merged.