From d700b53021de5036230cf43fbdf8dac67f4adc3e 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 | 65 +++++++++++++++++-- .../spree/admin/base_controller_spec.rb | 34 ++++++++-- .../controllers/spree/base_controller_spec.rb | 34 ++++++++-- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/lib/spree/auth/engine.rb b/lib/spree/auth/engine.rb index 2a65eb55..3c822453 100644 --- a/lib/spree/auth/engine.rb +++ b/lib/spree/auth/engine.rb @@ -27,13 +27,44 @@ class Engine < Rails::Engine end def self.prepare_backend + + require 'byebug' + byebug + + 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 + 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 + + 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 + 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 + + redirect_to spree.admin_login_path + end end end end @@ -42,10 +73,36 @@ 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 + 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 + + 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 + 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 + + 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..24483a99 100644 --- a/spec/controllers/spree/admin/base_controller_spec.rb +++ b/spec/controllers/spree/admin/base_controller_spec.rb @@ -11,16 +11,38 @@ def index; authorize!(:read, :something); 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..3010ba36 100644 --- a/spec/controllers/spree/base_controller_spec.rb +++ b/spec/controllers/spree/base_controller_spec.rb @@ -11,16 +11,38 @@ def index; authorize!(:read, :something); 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