Devise-specific: include authenticatable_salt as part of original_user_scope_identifier
etipton opened this issue · 2 comments
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.
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.
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.