From 306558549854b5287572d143343281cbfcc38291 Mon Sep 17 00:00:00 2001 From: Chris Roos Date: Tue, 17 Oct 2023 13:37:17 +0100 Subject: [PATCH] Enable exceptions on open redirects Raising an exception on open redirects is the default behaviour in Rails 7[1]. We disabled it in ad01e1c5d6703e35b591bb349a69bdd0e4f4d5f2 (15 Mar 2022) with a comment saying: > allow open redirects, necessary for SSO My guess is that this was required for the `/auth/gds/sign_out` path provided by gds-sso as that path redirects to Signon (i.e. a different host). The gds-sso Gem was updated in PR 250[2] (merged on 29 Mar 2022) to explicitly allow redirecting to a different host in `/auth/gds/sign_out`. I believe that change in gds-sso means we can safely remove this line. I've checked that signing in and out continues to work as expected with this line removed. I think the only risk of something breaking with this change is if `session["redirect_to"]` somehow contained a URL and not just the requested path that's stored in `GDS::SSO::FailureApp#store_location`[3]. I don't think we'd expect that to happen so it seems reasonable to raise an UnsafeRedirectError if it does. [1]: https://guides.rubyonrails.org/configuring.html#config-action-controller-raise-on-open-redirects [2]: https://github.com/alphagov/gds-sso/pull/250 [3]: https://github.com/alphagov/gds-sso/blob/v18.1.0/lib/gds-sso/failure_app.rb#L47 --- config/application.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index fd024832..935cab67 100644 --- a/config/application.rb +++ b/config/application.rb @@ -14,7 +14,6 @@ module AuthenticatingProxy class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. config.load_defaults 7.0 - Rails.application.config.action_controller.raise_on_open_redirects = false # Configuration for the application, engines, and railties goes here. #