Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When a controller action fails to be authorized, redirect back or default to /unauthorized #3118

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ Spree.config do |config|
# See https://github.com/solidusio/solidus/pull/3456 for more info.
config.raise_with_invalid_currency = false

# Set this configuration to false to always redirect the user to
# /unauthorized when needed, without trying to redirect them to
# their previous location first.
config.redirect_back_on_unauthorized = true

# Set this configuration to `false` to avoid running validations when
# updating an order. Be careful since you can end up having inconsistent
# data in your database turning it on.
Expand Down
6 changes: 6 additions & 0 deletions core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ class AppConfiguration < Preferences::Configuration
# @return [Boolean] (default: +true+)
preference :raise_with_invalid_currency, :boolean, default: true

# @!attribute [rw] redirect_back_on_unauthorized
# Whether to try to redirect users back when they try to access
# unauthorized routes, before redirect them to /unauthorized.
# @return [Boolean] (default: +false+)
preference :redirect_back_on_unauthorized, :boolean, default: false

# @!attribute [rw] require_master_price
# @return [Boolean] Require a price on the master variant of a product (default: +true+)
preference :require_master_price, :boolean, default: true
Expand Down
17 changes: 15 additions & 2 deletions core/lib/spree/core/controller_helpers/auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Auth
# @!attribute [rw] unauthorized_redirect
# @!scope class
# Extension point for overriding behaviour of access denied errors.
# Default behaviour is to redirect to "/unauthorized" with a flash
# Default behaviour is to redirect back or to "/unauthorized" with a flash
# message.
# @return [Proc] action to take when access denied error is raised.

Expand All @@ -22,7 +22,20 @@ module Auth
class_attribute :unauthorized_redirect
self.unauthorized_redirect = -> do
flash[:error] = I18n.t('spree.authorization_failure')
redirect_to "/unauthorized"
if Spree::Config.redirect_back_on_unauthorized
redirect_back(fallback_location: "/unauthorized")
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 "/unauthorized"
end
end

rescue_from CanCan::AccessDenied do
Expand Down
1 change: 1 addition & 0 deletions core/lib/spree/testing_support/dummy_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class Application < ::Rails::Application
Spree.config do |config|
config.mails_from = "store@example.com"
config.raise_with_invalid_currency = false
config.redirect_back_on_unauthorized = true
config.run_order_validations_on_order_updater = true
config.use_combined_first_and_last_name_in_address = true

Expand Down
22 changes: 22 additions & 0 deletions core/spec/lib/spree/core/controller_helpers/auth_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,26 @@ def controller.index
expect(controller.try_spree_current_user).to eq nil
end
end

describe '#unauthorized_redirect' do
before do
def controller.index
authorize!(:read, :something)
end
end

context "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

it "redirects to unauthorized" do
get :index
expect(response).to redirect_to('/unauthorized')
end
end
end