From 10df4eebce805989f155254832f82b645a28226a Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Wed, 20 May 2020 17:03:07 +0200 Subject: [PATCH] Try to redirect back on unauthorized accesses Only when the `redirect_back_on_unauthorized` preference exists and is set to true. This preference has been introduced in core with solidusio/solidus#3118 and we can rely on that preference to drive the behavior change here as well. The extra if Spree::Config.respond_to?(:redirect_back_on_unauthorized) check might seem useless but it's needed to avoid printing this deprecation warning on Solidus versions that still do not have the preference. If the Solidus verion used does not have the preference yet, the old behavior will be preserved. --- lib/spree/auth/engine.rb | 68 +++++++++++++++++-- .../spree/admin/base_controller_spec.rb | 38 +++++++++-- .../controllers/spree/base_controller_spec.rb | 38 +++++++++-- 3 files changed, 128 insertions(+), 16 deletions(-) diff --git a/lib/spree/auth/engine.rb b/lib/spree/auth/engine.rb index 2a65eb55..baf10d77 100644 --- a/lib/spree/auth/engine.rb +++ b/lib/spree/auth/engine.rb @@ -30,10 +30,40 @@ def self.prepare_backend Spree::Admin::BaseController.unauthorized_redirect = -> do if try_spree_current_user flash[:error] = I18n.t('spree.authorization_failure') - redirect_to spree.admin_unauthorized_path + + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) && Spree::Config.redirect_back_on_unauthorized + redirect_back(fallback_location: spree.admin_unauthorized_path) + else + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) + Spree::Deprecation.warn <<-WARN.strip_heredoc, caller + Having Spree::Config.redirect_back_on_unauthorized set + to `false` is deprecated and will not be supported in Solidus 3.0. + Please change this configuration to `true` and be sure that your + application does not break trying to redirect back when there is + an unauthorized access. + WARN + end + + redirect_to spree.admin_unauthorized_path + end else store_location - redirect_to spree.admin_login_path + + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) && Spree::Config.redirect_back_on_unauthorized + redirect_back(fallback_location: spree.admin_login_path) + else + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) + Spree::Deprecation.warn <<-WARN.strip_heredoc, caller + Having Spree::Config.redirect_back_on_unauthorized set + to `false` is deprecated and will not be supported in Solidus 3.0. + Please change this configuration to `true` and be sure that your + application does not break trying to redirect back when there is + an unauthorized access. + WARN + end + + redirect_to spree.admin_login_path + end end end end @@ -42,10 +72,40 @@ def self.prepare_frontend Spree::BaseController.unauthorized_redirect = -> do if try_spree_current_user flash[:error] = I18n.t('spree.authorization_failure') - redirect_to spree.unauthorized_path + + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) && Spree::Config.redirect_back_on_unauthorized + redirect_back(fallback_location: spree.unauthorized_path) + else + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) + Spree::Deprecation.warn <<-WARN.strip_heredoc, caller + Having Spree::Config.redirect_back_on_unauthorized set + to `false` is deprecated and will not be supported in Solidus 3.0. + Please change this configuration to `true` and be sure that your + application does not break trying to redirect back when there is + an unauthorized access. + WARN + end + + redirect_to spree.unauthorized_path + end else store_location - redirect_to spree.login_path + + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) && Spree::Config.redirect_back_on_unauthorized + redirect_back(fallback_location: spree.login_path) + else + if Spree::Config.respond_to?(:redirect_back_on_unauthorized) + Spree::Deprecation.warn <<-WARN.strip_heredoc, caller + Having Spree::Config.redirect_back_on_unauthorized set + to `false` is deprecated and will not be supported in Solidus 3.0. + Please change this configuration to `true` and be sure that your + application does not break trying to redirect back when there is + an unauthorized access. + WARN + end + + redirect_to spree.login_path + end end end end diff --git a/spec/controllers/spree/admin/base_controller_spec.rb b/spec/controllers/spree/admin/base_controller_spec.rb index 9f9a0aa4..5b81942a 100644 --- a/spec/controllers/spree/admin/base_controller_spec.rb +++ b/spec/controllers/spree/admin/base_controller_spec.rb @@ -8,19 +8,45 @@ def index; authorize!(:read, :something); end end + before do + stub_spree_preferences(Spree::Config, redirect_back_on_unauthorized: true) + end + context "when user is logged in" do before { sign_in(create(:user)) } - it "redirects to unauthorized path" do - get :index - expect(response).to redirect_to(spree.admin_unauthorized_path) + context "when http_referrer is not present" do + it "redirects to unauthorized path" do + get :index + expect(response).to redirect_to(spree.admin_unauthorized_path) + end + end + + context "when http_referrer is present" do + before { request.env['HTTP_REFERER'] = '/redirect' } + + it "redirects back" do + get :index + expect(response).to redirect_to('/redirect') + end end end context "when user is not logged in" do - it "redirects to login path" do - get :index - expect(response).to redirect_to(spree.admin_login_path) + context "when http_referrer is not present" do + it "redirects to login path" do + get :index + expect(response).to redirect_to(spree.admin_login_path) + end + end + + context "when http_referrer is present" do + before { request.env['HTTP_REFERER'] = '/redirect' } + + it "redirects back" do + get :index + expect(response).to redirect_to('/redirect') + end end end end diff --git a/spec/controllers/spree/base_controller_spec.rb b/spec/controllers/spree/base_controller_spec.rb index 29cd8d1b..9dc565cd 100644 --- a/spec/controllers/spree/base_controller_spec.rb +++ b/spec/controllers/spree/base_controller_spec.rb @@ -8,19 +8,45 @@ def index; authorize!(:read, :something); end end + before do + stub_spree_preferences(Spree::Config, redirect_back_on_unauthorized: true) + end + context "when user is logged in" do before { sign_in(create(:user)) } - it "redirects to unauthorized path" do - get :index - expect(response).to redirect_to(spree.unauthorized_path) + context "when http_referrer is not present" do + it "redirects to unauthorized path" do + get :index + expect(response).to redirect_to(spree.unauthorized_path) + end + end + + context "when http_referrer is present" do + before { request.env['HTTP_REFERER'] = '/redirect' } + + it "redirects back" do + get :index + expect(response).to redirect_to('/redirect') + end end end context "when user is not logged in" do - it "redirects to login path" do - get :index - expect(response).to redirect_to(spree.login_path) + context "when http_referrer is not present" do + it "redirects to login path" do + get :index + expect(response).to redirect_to(spree.login_path) + end + end + + context "when http_referrer is present" do + before { request.env['HTTP_REFERER'] = '/redirect' } + + it "redirects back" do + get :index + expect(response).to redirect_to('/redirect') + end end end end