Permalink
Browse files

NullSessionHash#destroy should be a no-op

Previously it was raising a NilException
  • Loading branch information...
jbaudanza committed Sep 19, 2013
1 parent 1ea4a89 commit 0f3124dd85b21abd7e1a5705f8574b62a6e46138
@@ -124,6 +124,9 @@ def initialize(env)
@loaded = true
end
+ # no-op
+ def destroy; end
+
def exists?
true
end
@@ -78,6 +78,11 @@ def encrypted
cookies.encrypted[:foo] = 'bar'
render :nothing => true
end
+
+ def try_to_reset_session
+ reset_session
+ render :nothing => true
+ end
end
class FreeCookieController < RequestForgeryProtectionControllerUsingResetSession
@@ -320,6 +325,11 @@ def setup
post :encrypted
assert_response :ok
end
+
+ test 'should allow reset_session' do
+ post :try_to_reset_session
+ assert_response :ok
+ end
end
class RequestForgeryProtectionControllerUsingExceptionTest < ActionController::TestCase

3 comments on commit 0f3124d

@jhilden

This comment has been minimized.

Show comment
Hide comment
@jhilden

jhilden Nov 4, 2013

Thanks for fixing this @jbaudanza
Do you have any idea when this will be part of a Rails release?

The previous behavior was causing the following issue: NoamB/sorcery#464

Thanks for fixing this @jbaudanza
Do you have any idea when this will be part of a Rails release?

The previous behavior was causing the following issue: NoamB/sorcery#464

@jbaudanza

This comment has been minimized.

Show comment
Hide comment
@jbaudanza

jbaudanza Nov 4, 2013

Contributor

@jhilden I don't. I'm not sure why it didn't go into 4.0.1.

Have you considered removing the protect_from_forgery filter from your sessions controller? If you're resetting the session anyway, there isn't much point in protecting it with an authenticity token.

Contributor

jbaudanza replied Nov 4, 2013

@jhilden I don't. I'm not sure why it didn't go into 4.0.1.

Have you considered removing the protect_from_forgery filter from your sessions controller? If you're resetting the session anyway, there isn't much point in protecting it with an authenticity token.

@jhilden

This comment has been minimized.

Show comment
Hide comment
@jhilden

jhilden Nov 5, 2013

That's a very valid point.

What does @kirs think about that? He probably has more experience in this area. Would it make sense to use skip_before_filter :protect_from_forgery, only: [:create, :destroy] in the sessions controller, since both will eventually call reset_session inside sorcery?

That's a very valid point.

What does @kirs think about that? He probably has more experience in this area. Would it make sense to use skip_before_filter :protect_from_forgery, only: [:create, :destroy] in the sessions controller, since both will eventually call reset_session inside sorcery?

Please sign in to comment.