diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c428021bcd..2b6582df78 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Change default redirection status code for non-GET/HEAD requests to 308 Permanent Redirect for `ActionDispatch::SSL`. + + *Alan Tan*, *Oz Ben-David* + * Fix `follow_redirect!` to follow redirection with same HTTP verb when following a 308 redirection. diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 6e490f5c9f..cd3883f0e2 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -56,7 +56,7 @@ module ActionDispatch { expires: HSTS_EXPIRES_IN, subdomains: true, preload: false } end - def initialize(app, redirect: {}, hsts: {}, secure_cookies: true) + def initialize(app, redirect: {}, hsts: {}, secure_cookies: true, ssl_default_redirect_status: nil) @app = app @redirect = redirect @@ -65,6 +65,7 @@ module ActionDispatch @secure_cookies = secure_cookies @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) + @ssl_default_redirect_status = ssl_default_redirect_status end def call(env) @@ -132,6 +133,8 @@ module ActionDispatch def redirection_status(request) if request.get? || request.head? 301 # Issue a permanent redirect via a GET request. + elsif @ssl_default_redirect_status + @ssl_default_redirect_status else 307 # Issue a fresh request redirect to preserve the HTTP method. end diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index 2989039c2d..0115c51298 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -21,7 +21,7 @@ class RedirectSSLTest < SSLTest end def assert_redirected(redirect: {}, from: "http://a/b?c=d", to: from.sub("http", "https")) - redirect = { status: 301, body: [] }.merge(redirect) + redirect = { body: [] }.merge(redirect) self.app = build_app ssl_options: { redirect: redirect } @@ -64,8 +64,22 @@ class RedirectSSLTest < SSLTest assert_post_redirected end - test "redirect with non-301 status" do - assert_redirected redirect: { status: 307 } + test "redirect with custom status" do + assert_redirected redirect: { status: 308 } + end + + test "redirect with ssl_default_redirect_status" do + self.app = build_app(ssl_options: { ssl_default_redirect_status: 308 }) + + get "http://a/b?c=d" + + assert_response 301 + assert_redirected_to "https://a/b?c=d" + + post "http://a/b?c=d" + + assert_response 308 + assert_redirected_to "https://a/b?c=d" end test "redirect with custom body" do diff --git a/guides/source/configuring.md b/guides/source/configuring.md index fefac22269..0923a9bf9c 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -628,6 +628,11 @@ Defaults to `'signed cookie'`. value of the `SameSite` attribute when setting cookies. Defaults to `nil`, which means the `SameSite` attribute is not added. +* `config.action_dispatch.ssl_default_redirect_status` configures the default + HTTP status code used when redirecting non-GET/HEAD requests from HTTP to HTTPS + in the `ActionDispatch::SSL` middleware. Defaults to `308` as defined in + https://tools.ietf.org/html/rfc7538. + * `ActionDispatch::Callbacks.before` takes a block of code to run before the request. * `ActionDispatch::Callbacks.after` takes a block of code to run after the request. diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 0fc3c74517..6305e12f67 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -176,6 +176,10 @@ This change is backwards compatible for the majority of applications, in which c Technically, however, controllers could configure `helpers_path` to point to a directoy in `$LOAD_PATH` that was not in the autoload paths. That use case is no longer supported out of the box. If the helper module is not autoloadable, the application is responsible for loading it before calling `helper`. +### Redirection to HTTPS from HTTP will now use the 308 HTTP status code + +The default HTTP status code used in `ActionDispatch::SSL` when redirecting non-GET/HEAD requests from HTTP to HTTPS has been changed to `308` as defined in https://tools.ietf.org/html/rfc7538. + Upgrading from Rails 5.2 to Rails 6.0 ------------------------------------- diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index ffe2c83c90..651a9999dc 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -175,6 +175,7 @@ module Rails if respond_to?(:action_dispatch) action_dispatch.cookies_same_site_protection = :lax + action_dispatch.ssl_default_redirect_status = 308 end if respond_to?(:action_controller) diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 563290da15..b645840f2e 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -16,7 +16,8 @@ module Rails middleware.use ::ActionDispatch::HostAuthorization, config.hosts, config.action_dispatch.hosts_response_app if config.force_ssl - middleware.use ::ActionDispatch::SSL, **config.ssl_options + middleware.use ::ActionDispatch::SSL, **config.ssl_options, + ssl_default_redirect_status: config.action_dispatch.ssl_default_redirect_status end middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt index d2db2e0548..f486d863dd 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt @@ -34,3 +34,7 @@ # Specify whether `ActiveSupport::TimeZone.utc_to_local` returns a time with an # UTC offset or a UTC time. # ActiveSupport.utc_to_local_returns_utc_offset_times = true + +# Change default HTTP status code to `308` when redirecting non-GET/HEAD requets +# to HTTPS in `ActionDispatch::SSL` middleware. +# Rails.application.config.action_dispatch.ssl_default_redirect_status = 308 diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index acaec7db4b..3dcf4b105c 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2347,6 +2347,28 @@ module ApplicationTests assert_equal :lax, Rails.application.config.action_dispatch.cookies_same_site_protection end + test "Rails.application.config.action_dispatch.ssl_default_redirect_status is 308 in 6.1 defaults" do + remove_from_config '.*config\.load_defaults.*\n' + add_to_config 'config.load_defaults "6.1"' + + app "production" + + assert_equal 308, Rails.application.config.action_dispatch.ssl_default_redirect_status + end + + test "Rails.application.config.action_dispatch.ssl_default_redirect_status can be configured in an initializer" do + remove_from_config '.*config\.load_defaults.*\n' + add_to_config 'config.load_defaults "6.0"' + + app_file "config/initializers/new_framework_defaults_6_1.rb", <<-RUBY + Rails.application.config.action_dispatch.ssl_default_redirect_status = 308 + RUBY + + app "production" + + assert_equal 308, Rails.application.config.action_dispatch.ssl_default_redirect_status + end + test "ActiveSupport.utc_to_local_returns_utc_offset_times is true in 6.1 defaults" do remove_from_config '.*config\.load_defaults.*\n' add_to_config 'config.load_defaults "6.1"' diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index dcec088991..2155973efc 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -145,7 +145,7 @@ module ApplicationTests add_to_config "config.ssl_options = { redirect: { host: 'example.com' } }" boot! - assert_equal [{ redirect: { host: "example.com" } }], Rails.application.middleware[2].args + assert_equal [{ redirect: { host: "example.com" }, ssl_default_redirect_status: 308 }], Rails.application.middleware[2].args end test "removing Active Record omits its middleware" do