From 53ddd046d996fb83e22d7c5fe52ea89dd9358d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 4 Sep 2020 14:45:44 +0200 Subject: [PATCH 01/10] Add "load_defaults" method and undo what it does 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. --- config/application.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/config/application.rb b/config/application.rb index b137dd5f6..6747e0880 100644 --- a/config/application.rb +++ b/config/application.rb @@ -8,9 +8,14 @@ Bundler.require(*Rails.groups) module Consul class Application < Rails::Application - # Settings in config/environments/* take precedence over those specified here. - # Application configuration should go into files in config/initializers - # -- all .rb files in that directory are automatically loaded. + config.load_defaults 5.0 + + # Overwrite Rails 5.0 defaults and use the options we used in Rails 4 + config.action_controller.per_form_csrf_tokens = nil + config.action_controller.forgery_protection_origin_check = nil + ActiveSupport.to_time_preserves_timezone = false + config.active_record.belongs_to_required_by_default = false + config.ssl_options = {} # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. From 611d729080742d9f1d95d2b2eb9533ea3bdd36de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 4 Sep 2020 16:10:01 +0200 Subject: [PATCH 02/10] Enable per form CSRF tokens 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. --- config/application.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 6747e0880..be07a620c 100644 --- a/config/application.rb +++ b/config/application.rb @@ -11,7 +11,6 @@ module Consul config.load_defaults 5.0 # Overwrite Rails 5.0 defaults and use the options we used in Rails 4 - config.action_controller.per_form_csrf_tokens = nil config.action_controller.forgery_protection_origin_check = nil ActiveSupport.to_time_preserves_timezone = false config.active_record.belongs_to_required_by_default = false From f1b38d20c193628216aec649bae3f46d7264ee37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 4 Sep 2020 16:10:22 +0200 Subject: [PATCH 03/10] Enable Origin header check in forgery protection 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. --- config/application.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index be07a620c..6c0b838f7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -11,7 +11,6 @@ module Consul config.load_defaults 5.0 # Overwrite Rails 5.0 defaults and use the options we used in Rails 4 - config.action_controller.forgery_protection_origin_check = nil ActiveSupport.to_time_preserves_timezone = false config.active_record.belongs_to_required_by_default = false config.ssl_options = {} From 0734e788bd7b27d675a158237b501749f45df2f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 4 Sep 2020 16:50:43 +0200 Subject: [PATCH 04/10] Preserve timezone when calling `to_time` 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. --- config/application.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 6c0b838f7..363e8160e 100644 --- a/config/application.rb +++ b/config/application.rb @@ -11,7 +11,6 @@ module Consul config.load_defaults 5.0 # Overwrite Rails 5.0 defaults and use the options we used in Rails 4 - ActiveSupport.to_time_preserves_timezone = false config.active_record.belongs_to_required_by_default = false config.ssl_options = {} From b1c112952f580a5fd2c7aa4ec8d9535a78b7cc32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 4 Sep 2020 17:22:33 +0200 Subject: [PATCH 05/10] Use Rails 5 default SSL options 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 --- config/application.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 363e8160e..cd56a56a2 100644 --- a/config/application.rb +++ b/config/application.rb @@ -12,7 +12,6 @@ module Consul # Overwrite Rails 5.0 defaults and use the options we used in Rails 4 config.active_record.belongs_to_required_by_default = false - config.ssl_options = {} # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. From 32f06ea7d9b14ca846814a1c9ace96884541d8ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 14:47:58 +0200 Subject: [PATCH 06/10] Keep belongs_to optional by default 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. --- config/application.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index cd56a56a2..cf73f4c96 100644 --- a/config/application.rb +++ b/config/application.rb @@ -10,7 +10,8 @@ module Consul class Application < Rails::Application config.load_defaults 5.0 - # Overwrite Rails 5.0 defaults and use the options we used in Rails 4 + # Keep belongs_to fields optional by default, because that's the way + # Rails 4 models worked config.active_record.belongs_to_required_by_default = false # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. From 5442ca7c542d4a1bfa7a26cbace200244208bd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 4 Sep 2020 22:16:55 +0200 Subject: [PATCH 07/10] Remove redundant configuration option 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. --- config/environments/development.rb | 4 ---- config/environments/preproduction.rb | 4 ---- config/environments/production.rb | 4 ---- config/environments/staging.rb | 4 ---- 4 files changed, 16 deletions(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 0e3e7f9d9..07840b58c 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -46,10 +46,6 @@ Rails.application.configure do # number of complex assets. config.assets.debug = true - # Asset digests allow you to set far-future HTTP expiration dates on all assets, - # yet still be able to expire them through the digest params. - config.assets.digest = true - # Adds additional error checking when serving assets at runtime. # Checks for improperly declared sprockets dependencies. # Raises helpful error messages. diff --git a/config/environments/preproduction.rb b/config/environments/preproduction.rb index 3c21e58c7..86926d740 100644 --- a/config/environments/preproduction.rb +++ b/config/environments/preproduction.rb @@ -36,10 +36,6 @@ Rails.application.configure do # Do not fallback to assets pipeline if a precompiled asset is missed. config.assets.compile = false - # Asset digests allow you to set far-future HTTP expiration dates on all assets, - # yet still be able to expire them through the digest params. - config.assets.digest = true - # `config.assets.precompile` and `config.assets.version` have moved to config/initializers/assets.rb # Specifies the header that your server uses for sending files. diff --git a/config/environments/production.rb b/config/environments/production.rb index 9fe36cf48..f0d84f33d 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -30,10 +30,6 @@ Rails.application.configure do # Do not fallback to assets pipeline if a precompiled asset is missed. config.assets.compile = false - # Asset digests allow you to set far-future HTTP expiration dates on all assets, - # yet still be able to expire them through the digest params. - config.assets.digest = true - # `config.assets.precompile` and `config.assets.version` have moved to config/initializers/assets.rb # Enable serving of images, stylesheets, and JavaScripts from an asset server. diff --git a/config/environments/staging.rb b/config/environments/staging.rb index c189f787c..c3eb3df2c 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -36,10 +36,6 @@ Rails.application.configure do # Do not fallback to assets pipeline if a precompiled asset is missed. config.assets.compile = false - # Asset digests allow you to set far-future HTTP expiration dates on all assets, - # yet still be able to expire them through the digest params. - config.assets.digest = true - # `config.assets.precompile` and `config.assets.version` have moved to config/initializers/assets.rb # Specifies the header that your server uses for sending files. From 00dc58f8b32af05f144550516b7c04c33410bef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 12:24:26 +0200 Subject: [PATCH 08/10] Use Rails 5.1 defaults and overwrite them This way we know what we need to do to fully upgrade to Rails 5.1. --- config/application.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index cf73f4c96..49e67ea01 100644 --- a/config/application.rb +++ b/config/application.rb @@ -8,12 +8,16 @@ Bundler.require(*Rails.groups) module Consul class Application < Rails::Application - config.load_defaults 5.0 + config.load_defaults 5.1 # Keep belongs_to fields optional by default, because that's the way # Rails 4 models worked config.active_record.belongs_to_required_by_default = false + # Overwrite Rails 5.1 defaults and use the options we used in Rails 5.0 + config.action_view.form_with_generates_remote_forms = false + config.assets.unknown_asset_fallback = true + # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. # config.time_zone = 'Central Time (US & Canada)' From 905ac48bb9fd727a189bca427902b47f27c793df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 14:31:58 +0200 Subject: [PATCH 09/10] Raise an exception when an asset is not found 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. --- config/application.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 49e67ea01..fa7c95470 100644 --- a/config/application.rb +++ b/config/application.rb @@ -16,7 +16,6 @@ module Consul # Overwrite Rails 5.1 defaults and use the options we used in Rails 5.0 config.action_view.form_with_generates_remote_forms = false - config.assets.unknown_asset_fallback = true # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. From 5c0ba0b04c03a420b655495dd7065b74fa4ee3d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 14:51:02 +0200 Subject: [PATCH 10/10] Generate local forms with form_with by default 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. --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index fa7c95470..feb4c8664 100644 --- a/config/application.rb +++ b/config/application.rb @@ -14,7 +14,7 @@ module Consul # Rails 4 models worked config.active_record.belongs_to_required_by_default = false - # Overwrite Rails 5.1 defaults and use the options we used in Rails 5.0 + # Use local forms with `form_with`, so it works like `form_for` config.action_view.form_with_generates_remote_forms = false # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.