From 7bb7548d00cf819a9ef54933054a523049354e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 16 Jun 2021 19:52:33 +0200 Subject: [PATCH] Respond with 403 when features are disabled When administrators disabled features and users tried to access them with the browser, we were responding with a 500 "Internal Server Error" page, which in my humble opinion was incorrect. There was no error at all; the server worked exactly as expected. I think a 403 "Forbidden" code is better; since that feature is disabled, we're refusing to let users access it. We could also respond with a 404 "Not found", although I wonder whether that'll be confusing when administrators temporarily or accidentally disable a feature. A similar thing might happen if we responded with a 410 "Gone" code. Actually this case might be more confusing since users aren't that familiar with this code. In any case, all these options are better than the 500 error. --- config/application.rb | 3 ++ public/403.html | 41 +++++++++++++++++++++++++ spec/system/admin/feature_flags_spec.rb | 4 +-- 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 public/403.html diff --git a/config/application.rb b/config/application.rb index 50576fe28..90bf6cd38 100644 --- a/config/application.rb +++ b/config/application.rb @@ -36,6 +36,9 @@ module Consul # in any CONSUL installations config.active_support.use_authenticated_message_encryption = false + # Handle custom exceptions + config.action_dispatch.rescue_responses["FeatureFlags::FeatureDisabled"] = :forbidden + # 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/public/403.html b/public/403.html new file mode 100644 index 000000000..7c0b688ef --- /dev/null +++ b/public/403.html @@ -0,0 +1,41 @@ + + + + Error 403 | Forbidden: Access disabled + + + + + +
+

403

+

Access to this page has been disabled by the administrators.

+
+ + diff --git a/spec/system/admin/feature_flags_spec.rb b/spec/system/admin/feature_flags_spec.rb index c114b87e0..5fd48a934 100644 --- a/spec/system/admin/feature_flags_spec.rb +++ b/spec/system/admin/feature_flags_spec.rb @@ -36,12 +36,12 @@ describe "Admin feature flags", :admin do visit budget_path(budget) - expect(page).to have_content "Internal server error" + expect(page).to have_title "Forbidden" visit admin_budgets_path expect(page).to have_current_path admin_budgets_path - expect(page).to have_content "Internal server error" + expect(page).to have_title "Forbidden" end scenario "Enable a disabled participatory process" do