Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

User session can be destroyed with a GET request #431

Closed
martinrehfeld opened this Issue Aug 12, 2010 · 7 comments

Comments

Projects
None yet
2 participants
Contributor

martinrehfeld commented Aug 12, 2010

I noticed that devise_for generates the destroy_user_session route for a GET request. I think this is problematic and should be limited to DELETE requests.

(Surprisingly I could not find any prior discussion on this).

Collaborator

carlosantoniodasilva commented Aug 12, 2010

Devise uses GET for signing out so you are not required to have javascript in your application to get it running.

Contributor

martinrehfeld commented Aug 12, 2010

I am all for providing usable sites even without javascript. This could yet be achieved in different ways while still using the REST verbs appropriately and not having any link prefetcher accidentally killing your session.

Solution 1: Use a (hidden) sign out form with a submit button triggering the DELETE request (or to be exact trigger a POST with a _method param of delete).

Solution 2: (If you want a sign out link in the first place) do use javascript to trigger the DELETE directly and as a non-JS fallback link to a sign out page with a confirmation message and the same form as in solution 1.

Collaborator

carlosantoniodasilva commented Aug 12, 2010

The problem with buttons is that they would have to be totally styled to appear like links, and I guess sign out is usually a link. Also, having an extra page would bring more complexibility to Devise to handle that in my opinion.
I just remembered we had a similar discussion on the mailing list some time ago:
http://groups.google.com/group/plataformatec-devise/browse_thread/thread/99d466710a95a72/965df9eac3afab89?lnk=gst&q=DELETE#965df9eac3afab89

Contributor

martinrehfeld commented Aug 12, 2010

Thanks for the pointer to the prior discussion.

Yet the fact remains that it is very problematic to kill the user session with a GET; GET by definition should not change state. So every UA must safely assume that following a GET/link does not do any harm.

Be it POST or DELETE instead is debatable of course. Yet other popular authentication solutions (Authlogic, RestfulAuthentication) seem to have silently agreed to use DELETE.

Collaborator

carlosantoniodasilva commented Aug 12, 2010

Based on the comments in this commit: http://github.com/plataformatec/devise/commit/2103a673f0e0e8be731613aee629e779434a20ea
A patch would be good to have it working with some sort of configuration on the method to sign out. Thanks for bringing this discussion back.

Collaborator

carlosantoniodasilva commented Aug 13, 2010

Great work! Thanks.

This issue was closed.

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