Update framework defaults to match those in Rails 5.1#2309
Update framework defaults to match those in Rails 5.1#2309benhalpern merged 6 commits intoforem:masterfrom rhymes:rhymes/update-framework-defaults-to-rails51
Conversation
Each form now has its own CSRF in addition to the global one. I've left in place the setInterval
|
I think there may be issues with our current use of Rails that get in the way of how we do stuff. Currently we can't send CSRF tokens over with typical page requests on articles because we cache a shared version for all users. If we sent the CSRF token it would cache for the first request and then leak to subsequent requests. We load CSRF async to account for this. I'm not totally clear on the consequences of the proposed changes, but I believe it would compromise this constraint eh? |
|
I didn't know that :( I've noticed the async load for the CSRF and I was wondering the reason why, thanks for the explanation. What the I'd keep |
|
@benhalpern I don't think there's an easy way to have per form CSRF tokens and caching, the same way you had to build this mechanism to get around of caching and tokens in the first place. I reverted the changes related to that and made sure it's explained clearly in the |
benhalpern
left a comment
There was a problem hiding this comment.
This looks good. Thanks. I think sticking with a previous convention is okay for now. We can brainstorm additional layers of security that might be possible down the road.
|
Thanks @benhalpern ! |
* master: (83 commits) Update gitdocs (forem#2500) Condense 'ask me anything' to 'ama' (forem#2428) [ci skip] Add user_signed_in? to cache key for styles (forem#2498) Added troubleshooting for byebug without readline issue (forem#2481) Fix some frontend linting issues (forem#2495) [ci skip] Fix <br/> in footer. (forem#2491) Remove extra param and add message for prefill (forem#2487) Make Cards Change Dynamically and add Reader/Follower Charts (forem#2488) Temporarily comment out random (forem#2486) Add nav buttons to pwa desktop (forem#2484) Feature/filtered charts (forem#2482) Add caching for historical data (forem#2476) There are installation sections for other OSes now. (forem#2480) Add inbox guidelines to users (forem#2473) Release Open Inbox (forem#2468) Convert underscores in article slugs properly (forem#2472) Update framework defaults to match those in Rails 5.1 (forem#2309) Enable random order for specs (forem#2466) [ci skip] forem#118 Allow users to embed Medium posts with Liquid Tags (forem#1161) Allow API to return top articles (forem#2469) ... # Conflicts: # Envfile # app/models/user.rb # db/schema.rb
What type of PR is this? (check all applicable)
Description
Now that the code base has been migrated to Rails 5.2 it should be safe to upgrade framework defaults to match those of the previous version. The new defaults for Rails 5.2, see new_framework_defaults_5_2.rb can be turned on individually later on.
Regarding the flipped flags:
to_time_preserves_timezoneis an old flag for Ruby 2.4 and date time conversionbelongs_toassociation is not explicitly declared as optional and missing during validation (I checked the code, all the optional associations seem to be correctly declared)form_with(unused in the source code) generate remote form by defaultI've also re-enabled two CSRF tokens that were disabled, let me know if they should be left alone (I've tested them manually and they worked)
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?