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

Devise-specific: include authenticatable_salt as part of original_user_scope_identifier #64

Open
etipton opened this issue Jan 5, 2015 · 2 comments

Comments

@etipton
Copy link
Contributor

etipton commented Jan 5, 2015

Devise stores three things in the session to identify and authenticate a record: scope, the record's db id, and the record's "authenticatable_salt" (a method defined by devise)

The "switch back" / original_user functionality should probably do the same thing (at least if the provider is Devise), to be consistent from a security standpoint.

If sessions are being stored securely (encrypted), then this isn't strictly necessary for security, but again, it's more about being consistent with the provider's approach.

The issue -- and the reason Devise does this, I believe -- is that if sessions aren't being stored securely -- which is a setting outside the scope of this gem -- then all an attacker would need to do is inject an easily-guessable value, e.g. "admin_1", for the "original_user_scope_identifier" key into a session. Luckily, Rails now defaults to encrypted session cookies so it's probably not a major concern.

@etipton
Copy link
Contributor Author

etipton commented Jan 5, 2015

Someone please correct me if wrong - I'm curious to know if I'm right about the whole point of authenticatable_salt, or if it is actually just overkill.

@sergey-alekseev
Copy link
Contributor

Had the same concern yesterday when took a look into original_user implementation.
So it's probably just a matter of setting session[:original_user_scope_identifier]. However you can't easily set session unless you know secret_key_base. Didn't spend enough time on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants