Allow for independent authenticity tokens per-scope #2642

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@avit

avit commented Sep 20, 2013

Confirming @markedmondson's issue #2640. Now with a workaround.

avit added some commits Sep 20, 2013

Allow for independent authenticity tokens per-scope
After authenticating, this resets the unauthenticated session's CSRF
token, and also resets it for the newly-authenticated scope. When
authenticating to a second scope, the CSRF token for the original
scope is not destroyed.
@avit

This comment has been minimized.

Show comment Hide comment
@avit

avit Sep 22, 2013

Owner

I see here that this doesn't get the right session scope... still needs work.

I see here that this doesn't get the right session scope... still needs work.

@avit

This comment has been minimized.

Show comment Hide comment
@avit

avit Sep 24, 2013

I think this approach is a dead-end: the authenticated user might be posting to an unauthenticated page: there's no way to tell which scope's csrf token should be used.

avit commented Sep 24, 2013

I think this approach is a dead-end: the authenticated user might be posting to an unauthenticated page: there's no way to tell which scope's csrf token should be used.

@avit

This comment has been minimized.

Show comment Hide comment
@avit

avit Nov 6, 2013

I think we can make this work if we prefix the token with the scope name, e.g. admin-a1b2c3d4e5 and then split it for comparison when receiving a post. I'll give this another shot.

avit commented Nov 6, 2013

I think we can make this work if we prefix the token with the scope name, e.g. admin-a1b2c3d4e5 and then split it for comparison when receiving a post. I'll give this another shot.

@avit avit reopened this Nov 6, 2013

@@ -26,7 +26,7 @@ def create_admin(options={})
admin = Admin.create!(
:email => options[:email] || 'admin@test.com',
:password => '123456', :password_confirmation => '123456',
- :active => options[:active]
+ :active => options[:active] || true

This comment has been minimized.

Show comment Hide comment
@beedub

beedub Apr 3, 2014

When options[:active] is false, it is overridden to true, which seems like the exact opposite of what you want.

[1] pry(main)> options = { key: false }
=> {:key=>false}
[2] pry(main)> options[:key] || true
=> true
@beedub

beedub Apr 3, 2014

When options[:active] is false, it is overridden to true, which seems like the exact opposite of what you want.

[1] pry(main)> options = { key: false }
=> {:key=>false}
[2] pry(main)> options[:key] || true
=> true

This comment has been minimized.

Show comment Hide comment
@avit

avit Apr 3, 2014

Thanks for the catch @beedub.

@josevalim do you think this approach is still worth pursuing given your comment about Rails 4 on #2640? I'm not familiar with what they're doing with sessions now.

@avit

avit Apr 3, 2014

Thanks for the catch @beedub.

@josevalim do you think this approach is still worth pursuing given your comment about Rails 4 on #2640? I'm not familiar with what they're doing with sessions now.

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Apr 3, 2014

Member

I would like to try the new approach and if that doesn't suffice we can come back to exploring this one. Thanks!

@josevalim

josevalim Apr 3, 2014

Member

I would like to try the new approach and if that doesn't suffice we can come back to exploring this one. Thanks!

@josevalim josevalim closed this Apr 3, 2014

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