From 3267c81ba00fab77c2f543813cd90359e03af05b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 15 May 2020 15:19:10 +0200 Subject: [PATCH 01/11] Upgrade to Rails 5.2 All the code in the `bin/` and the `config/` folder has been generated running `rake app:update`, except the `escape_javascript_fix` file, which we've removed since the code there is already included in Rails 5.2. --- .rubocop.yml | 1 - Gemfile | 2 +- Gemfile.lock | 85 ++++++++++--------- bin/bundle | 2 +- bin/setup | 3 +- bin/update | 3 +- config/environments/development.rb | 8 +- config/environments/test.rb | 2 +- .../application_controller_renderer.rb | 10 ++- .../initializers/content_security_policy.rb | 25 ++++++ config/initializers/escape_javascript_fix.rb | 25 ------ .../new_framework_defaults_5_2.rb | 38 +++++++++ spec/rails_helper.rb | 12 --- spec/spec_helper.rb | 8 +- 14 files changed, 128 insertions(+), 96 deletions(-) create mode 100644 config/initializers/content_security_policy.rb delete mode 100644 config/initializers/escape_javascript_fix.rb create mode 100644 config/initializers/new_framework_defaults_5_2.rb diff --git a/.rubocop.yml b/.rubocop.yml index 2cf085d92..9941c7659 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -253,7 +253,6 @@ Rails/OutputSafety: Severity: warning Exclude: - app/helpers/text_with_links_helper.rb - - config/initializers/escape_javascript_fix.rb Rails/PluralizationGrammar: Enabled: true diff --git a/Gemfile b/Gemfile index 806cd94c5..32e1f6f0a 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source "https://rubygems.org" -gem "rails", "5.1.7" +gem "rails", "5.2.4.4" gem "acts-as-taggable-on", "~> 6.5.0" gem "acts_as_votable", "~> 0.12.1" diff --git a/Gemfile.lock b/Gemfile.lock index 7c6df00e9..89922ea49 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,39 +10,43 @@ GEM remote: https://rubygems.org/ remote: https://rails-assets.org/ specs: - actioncable (5.1.7) - actionpack (= 5.1.7) + actioncable (5.2.4.4) + actionpack (= 5.2.4.4) nio4r (~> 2.0) - websocket-driver (~> 0.6.1) - actionmailer (5.1.7) - actionpack (= 5.1.7) - actionview (= 5.1.7) - activejob (= 5.1.7) + websocket-driver (>= 0.6.1) + actionmailer (5.2.4.4) + actionpack (= 5.2.4.4) + actionview (= 5.2.4.4) + activejob (= 5.2.4.4) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.1.7) - actionview (= 5.1.7) - activesupport (= 5.1.7) - rack (~> 2.0) + actionpack (5.2.4.4) + actionview (= 5.2.4.4) + activesupport (= 5.2.4.4) + rack (~> 2.0, >= 2.0.8) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.1.7) - activesupport (= 5.1.7) + actionview (5.2.4.4) + activesupport (= 5.2.4.4) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.1.7) - activesupport (= 5.1.7) + activejob (5.2.4.4) + activesupport (= 5.2.4.4) globalid (>= 0.3.6) - activemodel (5.1.7) - activesupport (= 5.1.7) - activerecord (5.1.7) - activemodel (= 5.1.7) - activesupport (= 5.1.7) - arel (~> 8.0) - activesupport (5.1.7) + activemodel (5.2.4.4) + activesupport (= 5.2.4.4) + activerecord (5.2.4.4) + activemodel (= 5.2.4.4) + activesupport (= 5.2.4.4) + arel (>= 9.0) + activestorage (5.2.4.4) + actionpack (= 5.2.4.4) + activerecord (= 5.2.4.4) + marcel (~> 0.3.1) + activesupport (5.2.4.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -73,7 +77,7 @@ GEM nokogiri ancestry (3.2.1) activerecord (>= 4.2.0) - arel (8.0.0) + arel (9.0.0) ast (2.4.1) audited (4.9.0) activerecord (>= 4.2, < 6.1) @@ -338,6 +342,8 @@ GEM nokogiri (>= 1.5.9) mail (2.7.1) mini_mime (>= 0.1.1) + marcel (0.3.3) + mimemagic (~> 0.3.2) mdl (0.11.0) kramdown (~> 2.3) kramdown-parser-gfm (~> 1.1) @@ -427,17 +433,18 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) - rails (5.1.7) - actioncable (= 5.1.7) - actionmailer (= 5.1.7) - actionpack (= 5.1.7) - actionview (= 5.1.7) - activejob (= 5.1.7) - activemodel (= 5.1.7) - activerecord (= 5.1.7) - activesupport (= 5.1.7) + rails (5.2.4.4) + actioncable (= 5.2.4.4) + actionmailer (= 5.2.4.4) + actionpack (= 5.2.4.4) + actionview (= 5.2.4.4) + activejob (= 5.2.4.4) + activemodel (= 5.2.4.4) + activerecord (= 5.2.4.4) + activestorage (= 5.2.4.4) + activesupport (= 5.2.4.4) bundler (>= 1.3.0) - railties (= 5.1.7) + railties (= 5.2.4.4) sprockets-rails (>= 2.0.0) rails-assets-leaflet (1.5.1) rails-assets-markdown-it (9.0.1) @@ -449,12 +456,12 @@ GEM rails-i18n (5.1.3) i18n (>= 0.7, < 2) railties (>= 5.0, < 6) - railties (5.1.7) - actionpack (= 5.1.7) - activesupport (= 5.1.7) + railties (5.2.4.4) + actionpack (= 5.2.4.4) + activesupport (= 5.2.4.4) method_source rake (>= 0.8.7) - thor (>= 0.18.1, < 2.0) + thor (>= 0.19.0, < 2.0) rainbow (3.0.0) rake (13.0.1) rb-fsevent (0.10.4) @@ -615,7 +622,7 @@ GEM nokogiri (~> 1.6) rubyzip (>= 1.3.0) selenium-webdriver (>= 3.0, < 4.0) - websocket-driver (0.6.5) + websocket-driver (0.7.3) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) whenever (1.0.0) @@ -694,7 +701,7 @@ DEPENDENCIES pg (~> 1.0.0) pg_search (~> 2.3.0) puma (~> 4.3.6) - rails (= 5.1.7) + rails (= 5.2.4.4) rails-assets-leaflet! rails-assets-markdown-it (~> 9.0.1)! recipient_interceptor (~> 0.2.0) diff --git a/bin/bundle b/bin/bundle index fe1874509..67efc37fb 100755 --- a/bin/bundle +++ b/bin/bundle @@ -1,3 +1,3 @@ #!/usr/bin/env ruby -ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", __FILE__) +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) load Gem.bin_path("bundler", "bundle") diff --git a/bin/setup b/bin/setup index 4ccb4facf..aa921f10d 100755 --- a/bin/setup +++ b/bin/setup @@ -1,10 +1,9 @@ #!/usr/bin/env ruby -require "pathname" require "fileutils" include FileUtils # path to your application root. -APP_ROOT = Pathname.new File.expand_path("../../", __FILE__) +APP_ROOT = File.expand_path("..", __dir__) def system!(*args) system(*args) || abort("\n== Command #{args} failed ==") diff --git a/bin/update b/bin/update index 770b84062..80e5352ad 100755 --- a/bin/update +++ b/bin/update @@ -1,10 +1,9 @@ #!/usr/bin/env ruby -require "pathname" require "fileutils" include FileUtils # path to your application root. -APP_ROOT = Pathname.new File.expand_path("../../", __FILE__) +APP_ROOT = File.expand_path("..", __dir__) def system!(*args) system(*args) || abort("\n== Command #{args} failed ==") diff --git a/config/environments/development.rb b/config/environments/development.rb index 07840b58c..646ee316d 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -13,12 +13,13 @@ Rails.application.configure do config.consider_all_requests_local = true # Enable/disable caching. By default caching is disabled. - if Rails.root.join("tmp/caching-dev.txt").exist? + # Run rails dev:cache to toggle caching. + if Rails.root.join("tmp", "caching-dev.txt").exist? config.action_controller.perform_caching = true config.cache_store = :memory_store config.public_file_server.headers = { - "Cache-Control" => "public, max-age=172800" + "Cache-Control" => "public, max-age=#{2.days.to_i}" } else config.action_controller.perform_caching = false @@ -41,6 +42,9 @@ Rails.application.configure do # Raise an error on page load if there are pending migrations. config.active_record.migration_error = :page_load + # Highlight code that triggered database queries in logs. + config.active_record.verbose_query_logs = true + # Debug mode disables concatenation and preprocessing of assets. # This option may cause significant delays in view rendering with a large # number of complex assets. diff --git a/config/environments/test.rb b/config/environments/test.rb index c7059f5c1..6ebbecbfc 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -20,7 +20,7 @@ Rails.application.configure do # Configure public file server for tests with Cache-Control for performance. config.public_file_server.enabled = true config.public_file_server.headers = { - "Cache-Control" => "public, max-age=3600" + "Cache-Control" => "public, max-age=#{1.hour.to_i}" } # Show full error reports and disable caching. diff --git a/config/initializers/application_controller_renderer.rb b/config/initializers/application_controller_renderer.rb index 51639b67a..89d2efab2 100644 --- a/config/initializers/application_controller_renderer.rb +++ b/config/initializers/application_controller_renderer.rb @@ -1,6 +1,8 @@ # Be sure to restart your server when you modify this file. -# ApplicationController.renderer.defaults.merge!( -# http_host: 'example.org', -# https: false -# ) +# ActiveSupport::Reloader.to_prepare do +# ApplicationController.renderer.defaults.merge!( +# http_host: 'example.org', +# https: false +# ) +# end diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb new file mode 100644 index 000000000..d3bcaa5ec --- /dev/null +++ b/config/initializers/content_security_policy.rb @@ -0,0 +1,25 @@ +# Be sure to restart your server when you modify this file. + +# Define an application-wide content security policy +# For further information see the following documentation +# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy + +# Rails.application.config.content_security_policy do |policy| +# policy.default_src :self, :https +# policy.font_src :self, :https, :data +# policy.img_src :self, :https, :data +# policy.object_src :none +# policy.script_src :self, :https +# policy.style_src :self, :https + +# # Specify URI for violation reports +# # policy.report_uri "/csp-violation-report-endpoint" +# end + +# If you are using UJS then enable automatic nonce generation +# Rails.application.config.content_security_policy_nonce_generator = -> request { SecureRandom.base64(16) } + +# Report CSP violations to a specified URI +# For further information see the following documentation: +# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy-Report-Only +# Rails.application.config.content_security_policy_report_only = true diff --git a/config/initializers/escape_javascript_fix.rb b/config/initializers/escape_javascript_fix.rb deleted file mode 100644 index 0f693fdf6..000000000 --- a/config/initializers/escape_javascript_fix.rb +++ /dev/null @@ -1,25 +0,0 @@ -# Code taken from https://github.com/rails/rails/security/advisories/GHSA-65cv-r6x7-79hv -# Remove this code after upgrading to Rails 5.2 -ActionView::Helpers::JavaScriptHelper::JS_ESCAPE_MAP.merge!( - { - "`" => "\\`", - "$" => "\\$" - } -) - -module ActionView::Helpers::JavaScriptHelper - alias :old_ej :escape_javascript - alias :old_j :j - - def escape_javascript(javascript) - javascript = javascript.to_s - if javascript.empty? - result = "" - else - result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|\342\200\251|[\n\r"']|[`]|[$])/u, JS_ESCAPE_MAP) - end - javascript.html_safe? ? result.html_safe : result - end - - alias :j :escape_javascript -end diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb new file mode 100644 index 000000000..c383d072b --- /dev/null +++ b/config/initializers/new_framework_defaults_5_2.rb @@ -0,0 +1,38 @@ +# Be sure to restart your server when you modify this file. +# +# This file contains migration options to ease your Rails 5.2 upgrade. +# +# Once upgraded flip defaults one by one to migrate to the new default. +# +# Read the Guide for Upgrading Ruby on Rails for more info on each option. + +# Make Active Record use stable #cache_key alongside new #cache_version method. +# This is needed for recyclable cache keys. +# Rails.application.config.active_record.cache_versioning = true + +# Use AES-256-GCM authenticated encryption for encrypted cookies. +# Also, embed cookie expiry in signed or encrypted cookies for increased security. +# +# This option is not backwards compatible with earlier Rails versions. +# It's best enabled when your entire app is migrated and stable on 5.2. +# +# Existing cookies will be converted on read then written with the new scheme. +# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true + +# Use AES-256-GCM authenticated encryption as default cipher for encrypting messages +# instead of AES-256-CBC, when use_authenticated_message_encryption is set to true. +# Rails.application.config.active_support.use_authenticated_message_encryption = true + +# Add default protection from forgery to ActionController::Base instead of in +# ApplicationController. +# Rails.application.config.action_controller.default_protect_from_forgery = true + +# Store boolean values are in sqlite3 databases as 1 and 0 instead of 't' and +# 'f' after migrating old data. +# Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true + +# Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. +# Rails.application.config.active_support.use_sha1_digests = true + +# Make `form_with` generate id attributes for any generated HTML tags. +# Rails.application.config.action_view.form_with_generates_ids = true diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 3de8fd85d..c4756dd02 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -19,18 +19,6 @@ Warden.test_mode! ActiveRecord::Migration.maintain_test_schema! -# Monkey patch from https://github.com/rails/rails/pull/32293 -# Remove when we upgrade to Rails 5.2 -require "action_dispatch/system_testing/test_helpers/setup_and_teardown" -module ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown - def after_teardown - take_failed_screenshot - Capybara.reset_sessions! - ensure - super - end -end - RSpec.configure do |config| config.infer_spec_type_from_file_location! config.after do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7aa19aac4..5a775065f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -82,12 +82,8 @@ RSpec.configure do |config| .to receive(:available_locales).and_return(I18n.available_locales.map(&:to_s)) end - config.before(:each, :with_frozen_time) do - travel_to Time.current # TODO: use `freeze_time` after migrating to Rails 5.2. - end - - config.after(:each, :with_frozen_time) do - travel_back + config.around(:each, :with_frozen_time) do |example| + freeze_time { example.run } end config.before(:each, :application_zone_west_of_system_zone) do From 2d9f6791056a97e0dd24a07ec90788b45e7ac9fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 17 May 2020 19:56:54 +0200 Subject: [PATCH 02/11] Use Rails 5.2 schema format --- db/schema.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index fa1e0aa14..31ebc6540 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,12 +10,12 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20200908084257) do +ActiveRecord::Schema.define(version: 2020_09_08_084257) do # These are extensions that must be enabled in order to support this database + enable_extension "pg_trgm" enable_extension "plpgsql" enable_extension "unaccent" - enable_extension "pg_trgm" create_table "active_poll_translations", id: :serial, force: :cascade do |t| t.integer "active_poll_id", null: false From 16c16e3cdfec21997d6a92b8ca34d6b28fb9bf7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 17 May 2020 21:10:09 +0200 Subject: [PATCH 03/11] Mark safe SQL with `Arel.sql` Rails 5.2 is raising a warning in some places: DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s). Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). IMHO this warning is simply wrong, since we're using known PostgreSQL functions like LOWER() or RANDOM(). AFAIK this code works without warnings in Rails 6.0 [1][2] However, since the warning is annoying, we need to take measures so our logs are clean. [1] https://github.com/rails/rails/commit/6c82b6c99d [2] https://github.com/rails/rails/commit/64d8c54e16 --- app/controllers/admin/geozones_controller.rb | 2 +- app/controllers/legislation/annotations_controller.rb | 2 +- app/models/application_record.rb | 4 ++-- app/models/comment.rb | 2 +- app/models/poll/question.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/geozones_controller.rb b/app/controllers/admin/geozones_controller.rb index 6e3b0843a..1b924d444 100644 --- a/app/controllers/admin/geozones_controller.rb +++ b/app/controllers/admin/geozones_controller.rb @@ -4,7 +4,7 @@ class Admin::GeozonesController < Admin::BaseController load_and_authorize_resource def index - @geozones = Geozone.all.order("LOWER(name)") + @geozones = Geozone.all.order(Arel.sql("LOWER(name)")) end def new diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index a238e064e..1d26211c2 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -61,7 +61,7 @@ class Legislation::AnnotationsController < Legislation::BaseController end def search - @annotations = @draft_version.annotations.order("LENGTH(quote) DESC") + @annotations = @draft_version.annotations.order(Arel.sql("LENGTH(quote) DESC")) annotations_hash = { total: @annotations.size, rows: @annotations } render json: annotations_hash.to_json(methods: :weight) end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 4c7aa15dc..50235a9ca 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -3,9 +3,9 @@ class ApplicationRecord < ActiveRecord::Base def self.sample(count = 1) if count == 1 - reorder("RANDOM()").first + reorder(Arel.sql("RANDOM()")).first else - reorder("RANDOM()").limit(count) + reorder(Arel.sql("RANDOM()")).limit(count) end end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 7af862a73..f6607a849 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -49,7 +49,7 @@ class Comment < ApplicationRecord scope :sort_by_most_voted, -> { order(confidence_score: :desc, created_at: :desc) } scope :sort_descendants_by_most_voted, -> { order(confidence_score: :desc, created_at: :asc) } - scope :sort_by_supports, -> { order("cached_votes_up - cached_votes_down DESC") } + scope :sort_by_supports, -> { order(Arel.sql("cached_votes_up - cached_votes_down DESC")) } scope :sort_by_newest, -> { order(created_at: :desc) } scope :sort_descendants_by_newest, -> { order(created_at: :desc) } diff --git a/app/models/poll/question.rb b/app/models/poll/question.rb index 3e95997df..6f489c8d8 100644 --- a/app/models/poll/question.rb +++ b/app/models/poll/question.rb @@ -28,7 +28,7 @@ class Poll::Question < ApplicationRecord scope :by_poll_id, ->(poll_id) { where(poll_id: poll_id) } - scope :sort_for_list, -> { order("poll_questions.proposal_id IS NULL", :created_at) } + scope :sort_for_list, -> { order(Arel.sql("poll_questions.proposal_id IS NULL"), :created_at) } scope :for_render, -> { includes(:author, :proposal) } def self.search(params) From e865a664da0160f517fdda14697888ae9a6db181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 16:15:54 +0200 Subject: [PATCH 04/11] Make form_with generate ID attributes Even if we don't use form_with, it makes sense to configure it to behave the same way form_for does. This is the default option in Rails 5.2 applications. IMHO it should have been the default option for Rails 5.1 too, since generally form inputs need an ID so they can easily be associated with a label. --- config/initializers/new_framework_defaults_5_2.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb index c383d072b..e549feebb 100644 --- a/config/initializers/new_framework_defaults_5_2.rb +++ b/config/initializers/new_framework_defaults_5_2.rb @@ -35,4 +35,4 @@ # Rails.application.config.active_support.use_sha1_digests = true # Make `form_with` generate id attributes for any generated HTML tags. -# Rails.application.config.action_view.form_with_generates_ids = true +Rails.application.config.action_view.form_with_generates_ids = true From e58bd7f6f94a70e8c3a99d70c36fb6f265dcf8bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 17:29:12 +0200 Subject: [PATCH 05/11] Remove SQLite 3 configuration option Since we don't support SQLite, we don't care about options related to this database. --- config/initializers/new_framework_defaults_5_2.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb index e549feebb..954155cd7 100644 --- a/config/initializers/new_framework_defaults_5_2.rb +++ b/config/initializers/new_framework_defaults_5_2.rb @@ -27,10 +27,6 @@ # ApplicationController. # Rails.application.config.action_controller.default_protect_from_forgery = true -# Store boolean values are in sqlite3 databases as 1 and 0 instead of 't' and -# 'f' after migrating old data. -# Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true - # Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. # Rails.application.config.active_support.use_sha1_digests = true From 305bf9161cf9631aa1a25f18b8e58f6c4dcc4ff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 17:51:56 +0200 Subject: [PATCH 06/11] Enable forgery protection in ActionController We were manually adding forgery protection to all our controllers, but in Rails 5.2 there's an option (enabled by default for new applications) which adds this protection to all controllers. --- app/controllers/application_controller.rb | 1 - app/controllers/management/base_controller.rb | 1 - app/controllers/management/sessions_controller.rb | 1 - config/initializers/new_framework_defaults_5_2.rb | 2 +- 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 32cdbb135..5b74ae2e7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -7,7 +7,6 @@ class ApplicationController < ActionController::Base include AccessDeniedHandler default_form_builder ConsulFormBuilder - protect_from_forgery with: :exception before_action :authenticate_http_basic, if: :http_basic_auth_site? diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index 6a2788718..d0180b0a7 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -2,7 +2,6 @@ class Management::BaseController < ActionController::Base include GlobalizeFallbacks layout "management" default_form_builder ConsulFormBuilder - protect_from_forgery with: :exception before_action :verify_manager before_action :set_locale diff --git a/app/controllers/management/sessions_controller.rb b/app/controllers/management/sessions_controller.rb index d2fdfe3eb..84d9d1265 100644 --- a/app/controllers/management/sessions_controller.rb +++ b/app/controllers/management/sessions_controller.rb @@ -4,7 +4,6 @@ class Management::SessionsController < ActionController::Base include GlobalizeFallbacks include AccessDeniedHandler default_form_builder ConsulFormBuilder - protect_from_forgery with: :exception def create destroy_session diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb index 954155cd7..a8b584561 100644 --- a/config/initializers/new_framework_defaults_5_2.rb +++ b/config/initializers/new_framework_defaults_5_2.rb @@ -25,7 +25,7 @@ # Add default protection from forgery to ActionController::Base instead of in # ApplicationController. -# Rails.application.config.action_controller.default_protect_from_forgery = true +Rails.application.config.action_controller.default_protect_from_forgery = true # Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. # Rails.application.config.active_support.use_sha1_digests = true From 6756a8881592999ae073cc07be90282b558d5023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 18:11:22 +0200 Subject: [PATCH 07/11] Enable authenticated cookie encryption This is the default encryption for cookies in Rails 5.2 applications. The reason it isn't enabled automatically for existing applications is these cookies are not compatible with running the application on several servers when some of them use Rails 4. Since this isn't our case (we don't support using different versions of CONSUL on different servers), and existing cookies are still read correctly, we can safely enable it. --- config/initializers/new_framework_defaults_5_2.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb index a8b584561..686b6fd4e 100644 --- a/config/initializers/new_framework_defaults_5_2.rb +++ b/config/initializers/new_framework_defaults_5_2.rb @@ -17,7 +17,7 @@ # It's best enabled when your entire app is migrated and stable on 5.2. # # Existing cookies will be converted on read then written with the new scheme. -# Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true +Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true # Use AES-256-GCM authenticated encryption as default cipher for encrypting messages # instead of AES-256-CBC, when use_authenticated_message_encryption is set to true. From 00a5dc921a8fa5edd781d09357763112fc9c355f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 18:34:37 +0200 Subject: [PATCH 08/11] Enable SHA1 digests This is enabled by default in Rails 5.2 applications. Note this change will cause all fragment caching to expire. We consider it acceptable considering the page where caching is most important (stats) is barely affected by this change, since this change only affects the view, and the time-consuming operations are cached in the model. Comments are actually affected, though, and pages with thousands of comments might take a few extra seconds to load the first time they're accessed after this change. We don't think this is going to be an issue on existing CONSUL installations. --- config/initializers/new_framework_defaults_5_2.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb index 686b6fd4e..893bd860f 100644 --- a/config/initializers/new_framework_defaults_5_2.rb +++ b/config/initializers/new_framework_defaults_5_2.rb @@ -28,7 +28,7 @@ Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = t Rails.application.config.action_controller.default_protect_from_forgery = true # Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. -# Rails.application.config.active_support.use_sha1_digests = true +Rails.application.config.active_support.use_sha1_digests = true # Make `form_with` generate id attributes for any generated HTML tags. Rails.application.config.action_view.form_with_generates_ids = true From f42effe9feca125da3d4a4cb1a43e9da4dd26141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 5 Sep 2020 18:49:00 +0200 Subject: [PATCH 09/11] Use Rails 5.2 defaults and overwrite them We can remove the `new_framework_defaults_5_2` file by using Rails 5.2 default options and overwriting the ones we haven't enabled. We're disabling `use_authenticated_message_encryption` because, even if we don't use it, some CONSUL installations might be using it, and enabling this options would make it harder to decrypt existing encrypted messages. And we're disabling `cache_versioning` until we verify our cache keeps working and expires as expected on production environments, particularly for stats. --- config/application.rb | 10 +++++- .../new_framework_defaults_5_2.rb | 34 ------------------- 2 files changed, 9 insertions(+), 35 deletions(-) delete mode 100644 config/initializers/new_framework_defaults_5_2.rb diff --git a/config/application.rb b/config/application.rb index feb4c8664..26cef7743 100644 --- a/config/application.rb +++ b/config/application.rb @@ -8,7 +8,7 @@ Bundler.require(*Rails.groups) module Consul class Application < Rails::Application - config.load_defaults 5.1 + config.load_defaults 5.2 # Keep belongs_to fields optional by default, because that's the way # Rails 4 models worked @@ -17,6 +17,14 @@ module Consul # Use local forms with `form_with`, so it works like `form_for` config.action_view.form_with_generates_remote_forms = false + # Keep disabling cache versioning until we verify it's compatible + # with `:dalli_store` and with the way we cache stats + config.active_record.cache_versioning = false + + # Keep using AES-256-CBC for message encryption in case it's used + # in any CONSUL installations + config.active_support.use_authenticated_message_encryption = false + # 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)' diff --git a/config/initializers/new_framework_defaults_5_2.rb b/config/initializers/new_framework_defaults_5_2.rb deleted file mode 100644 index 893bd860f..000000000 --- a/config/initializers/new_framework_defaults_5_2.rb +++ /dev/null @@ -1,34 +0,0 @@ -# Be sure to restart your server when you modify this file. -# -# This file contains migration options to ease your Rails 5.2 upgrade. -# -# Once upgraded flip defaults one by one to migrate to the new default. -# -# Read the Guide for Upgrading Ruby on Rails for more info on each option. - -# Make Active Record use stable #cache_key alongside new #cache_version method. -# This is needed for recyclable cache keys. -# Rails.application.config.active_record.cache_versioning = true - -# Use AES-256-GCM authenticated encryption for encrypted cookies. -# Also, embed cookie expiry in signed or encrypted cookies for increased security. -# -# This option is not backwards compatible with earlier Rails versions. -# It's best enabled when your entire app is migrated and stable on 5.2. -# -# Existing cookies will be converted on read then written with the new scheme. -Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true - -# Use AES-256-GCM authenticated encryption as default cipher for encrypting messages -# instead of AES-256-CBC, when use_authenticated_message_encryption is set to true. -# Rails.application.config.active_support.use_authenticated_message_encryption = true - -# Add default protection from forgery to ActionController::Base instead of in -# ApplicationController. -Rails.application.config.action_controller.default_protect_from_forgery = true - -# Use SHA-1 instead of MD5 to generate non-sensitive digests, such as the ETag header. -Rails.application.config.active_support.use_sha1_digests = true - -# Make `form_with` generate id attributes for any generated HTML tags. -Rails.application.config.action_view.form_with_generates_ids = true From 7496c1bcb91a9a6697bdaf2c36c9f6c1ae31e93c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 6 Sep 2020 17:23:27 +0200 Subject: [PATCH 10/11] Disable ActiveStorage routes We aren't using ActiveStorage, but Rails was including its routes anyway. In Rails 6.1 there will be an option to disable these routes [1], but for now we're changing the line requiring "rails/all" to the values generated by a new Rails application with the --skip-active-storage flag. [1] https://github.com/rails/rails/commit/3cf65bcb8 --- config/application.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 26cef7743..819918788 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,6 +1,17 @@ require_relative "boot" -require "rails/all" +require "rails" +# Pick the frameworks you want: +require "active_model/railtie" +require "active_job/railtie" +require "active_record/railtie" +# require "active_storage/engine" +require "action_controller/railtie" +require "action_mailer/railtie" +require "action_view/railtie" +require "action_cable/engine" +require "sprockets/railtie" +require "rails/test_unit/railtie" # Require the gems listed in Gemfile, including any gems # you've limited to :test, :development, or :production. From 040ec9f588ec7be9b0ed4d7d951d9789b9d289da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 7 Oct 2020 13:35:51 +0200 Subject: [PATCH 11/11] Use `be_successful` instead of `be_success` We were getting a deprecation message in Rails 5.2: > The success? predicate is deprecated and will be removed in Rails 6.0. > Please use successful? as provided by Rack::Response::Helpers. --- spec/controllers/legislation/processes_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/legislation/processes_controller_spec.rb b/spec/controllers/legislation/processes_controller_spec.rb index c15a293f3..98dd33ec9 100644 --- a/spec/controllers/legislation/processes_controller_spec.rb +++ b/spec/controllers/legislation/processes_controller_spec.rb @@ -8,6 +8,6 @@ describe Legislation::ProcessesController do get :summary, params: { id: legislation_process, format: :xlsx } - expect(response).to be_success + expect(response).to be_successful end end