CSRF protection should rescue exception not extend #14786
Conversation
I think the changes to the default behaviour mean that rails will throw an exception when an invalid authenticity token is found. The previous proposed code of calling super then sign_out meant that sign_out was never reached - the exception handler never returned. I think the best approach now is to catch the exception, although I'm not 100% certain on that.
|
Guides should mention what is correct with the default behavior. If it is |
|
But in this case the code before it is not using the |
Extend previous changes, include the default line from the application controller that new rails applications are created with: protect_from_forgery with: :exception Minor wording changes to align.
|
Agree. Extended to align with your suggestion. |
|
On a related matter, the security guide makes no mention of setting encrypted_cookie_salt or encrypted_signed_cookie_salt to values other than the default. It seems likely that this would be a good idea (there seems little point having a salt with a known value), but I haven't seen anything anywhere on the web that recommends it. Is this an oversight? |
|
salt can be a known value without problem. But if you want to have be extra cautious you can set. I think we don't need to recommend changing it on the guide. |
CSRF protection should rescue exception not extend
I think the changes to the default behaviour mean that rails will throw an exception when an invalid authenticity token is found. The previous proposed code of calling super then sign_out meant that sign_out was never reached - the exception handler never returned.
I think the best approach now is to catch the exception, although I'm not 100% certain on that. I've handled this by proposing the documentation change, rather than just asking the question - feel free to reject if this isn't the right solution.