Use Rails 5 default settings #4141
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
javierm
force-pushed
the
load_defaults
branch
from
ea6bb64
to
78f36c2
Compare
The goal here is to have a notion on what the defaults are in a Rails 5 application, know why our application is working in a different way (it's because these defaults aren't loaded in an application which was originally developed using Rails 4), and have an explicit list of things we are overwriting. Furthermore, running the `app:update` rake task to upgrade to Rails 5.2 will by default add the line loading default options for Rails 5.0, so by adopting those default options we prevent accidental mistakes when upgrading. We'll have to review these items and see which ones can be changed to their default values for Rails 5 applications.
This is the default for new Rails application, and adds an extra layer of security since now the token will only be valid for its action, and so attackers managing to change the form action will not do any harm since the CSRF token will not work for the attackers' action. Note that we've had InvalidAuthenticityToken exceptions for years; if we keep getting them, chances are this change is *not* related.
This is the default in Rails 5 applications. This option is not enabled by default in existing applications because it would break applications running on several domains and doing POST requests between them or running a reverse proxy that rewrites the Host header. Since those aren't our cases, it's safe to enable it.
Quoting the Rails DateAndTime::Compatibility module: > With Ruby 2.4+ the default for +to_time+ changed from > converting to the local system time, to preserving the offset > of the receiver. For backwards compatibility we're overriding > this behavior We don't need backwards compatibility in our application because we aren't converting any time objects to the local system timezone but use the application timezone all the time instead.
The default options (which apply when `force_ssl` is set, which is the
default in CONSUL) are `{ hsts: { subdomains: true } }`, which means we
tell browsers to apply our SSL settings to subdomains as well [1].
CONSUL installations implementing multitenancy with subdomains will
benefit from this change.
[1] https://api.rubyonrails.org/classes/ActionDispatch/SSL.html
Changing it would mean reviewing and changing all our existing models, and some of them might be tricky (like our Document and Image models, which only validate certain associations in some cases), so we're keeping it the way it's been until now.
This option was added by Rails 4 new application generator. However, the `assets.digest` option is set to true by default, and recent Rails versions don't even add this option to the environment files.
This way we know what we need to do to fully upgrade to Rails 5.1.
This is the default in Rails 5.1 applications. If we want to use an asset in the public folder, we need to add the `public_folder: true` option, making it clear that we don't expect the asset to be in the asset pipeline. Since we don't use `asset_path` to reference assets in the public folder, we can safely disable the `unknown_asset_fallback` option.
We're not replacing `form_for` with `form_with` for now, and even if we did, most of our forms are not remote, so making them remote by default would be inconvenient.
taitus
approved these changes
Oct 6, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives