From 6c20ca03cd22e10a53a500d210110f507547e054 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 11 Oct 2019 17:20:35 +0200 Subject: [PATCH] Add UserReturnToStorer service object Spree::UserReturnToStorer can be used to selectively store the current path in `session[:spree_user_return_to]`. This object allows easy extension/change of behavior by using a list of rules. When at least one rule is matched, the request path is not stored in session. The class ships with the default rule AutenticationRule, that was extacted from Spree::Core::ControllerHelpers::Auth#store_location --- .../app/models/spree/user_return_to_storer.rb | 54 +++++++++++++++++ .../rules/authentication_rule.rb | 35 +++++++++++ core/lib/spree/app_configuration.rb | 4 ++ .../rules/authentication_rule_spec.rb | 30 ++++++++++ .../spree/user_return_to_storer_spec.rb | 59 +++++++++++++++++++ 5 files changed, 182 insertions(+) create mode 100644 core/app/models/spree/user_return_to_storer.rb create mode 100644 core/app/models/spree/user_return_to_storer/rules/authentication_rule.rb create mode 100644 core/spec/models/spree/user_return_to_storer/rules/authentication_rule_spec.rb create mode 100644 core/spec/models/spree/user_return_to_storer_spec.rb diff --git a/core/app/models/spree/user_return_to_storer.rb b/core/app/models/spree/user_return_to_storer.rb new file mode 100644 index 00000000000..1605a60f6b9 --- /dev/null +++ b/core/app/models/spree/user_return_to_storer.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Spree + # This service object is responsible for storing the current path into + # into `session[:spree_user_return_to]` for redirects after successful + # user/admin authentication. + class UserReturnToStorer + class Configuration + def rules + @rules ||= ::Spree::Core::ClassConstantizer::Set.new.tap do |set| + set << 'Spree::UserReturnToStorer::Rules::AuthenticationRule' + end + end + end + + # Lists all the rules that will be evaluated before storing the + # current path value into the session. + # + # @return [Spree::Core::ClassConstantizer::Set] a set of rules + # that, when matched, will prevent session[:spree_user_return_to] + # to be set + # + # @example This method can be used also to add more rules + # Spree::UserReturnToStorer.rules << 'CustomRule' + # + # @examle it can be used also removing unwanted rules + # Spree::UserReturnToStorer.rules.delete('CustomRule') + # + def self.rules + Spree::Config.user_return_to_storer.rules + end + + # @param controller [ApplicationController] an instance of ApplicationController + # or its subclasses. The controller will be passed to each rule for matching. + def initialize(controller) + @controller = controller + end + + # Stores into session[:spree_user_return_to] the request full path for + # future redirects (to be used after successful authentication). When + # there is a rule match then the request full path is not stored. + def store_location + return if self.class.rules.any? { |rule| rule.match? controller } + + session[:spree_user_return_to] = request.fullpath.gsub('//', '/') + end + + private + + attr_reader :controller + + delegate :session, :request, to: :controller + end +end diff --git a/core/app/models/spree/user_return_to_storer/rules/authentication_rule.rb b/core/app/models/spree/user_return_to_storer/rules/authentication_rule.rb new file mode 100644 index 00000000000..f6baf8a9704 --- /dev/null +++ b/core/app/models/spree/user_return_to_storer/rules/authentication_rule.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Spree + class UserReturnToStorer + module Rules + # This is the basic rule that ships with Solidus that avoids storing in + # session the current path for login/loout/signup routes, avoiding possibly + # infinte redirects. + module AuthenticationRule + AUTHENTICATION_ROUTES = %w[spree_signup_path spree_login_path spree_logout_path] + + extend self + + def match?(controller) + full_path = controller.request.fullpath + disallowed_urls(controller).include?(full_path) + end + + private + + def disallowed_urls(controller) + @disallowed_urls ||= begin + [].tap do |disallowed_urls| + AUTHENTICATION_ROUTES.each do |route| + if controller.respond_to?(route) + disallowed_urls << controller.send(route) + end + end + end.map! { |url| url[/\/\w+$/] } + end + end + end + end + end +end diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 4837c0c0559..3e58f3a217a 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -477,6 +477,10 @@ def events @events_configuration ||= Spree::Event::Configuration.new end + def user_return_to_storer + @user_return_to_storer_configuration ||= Spree::UserReturnToStorer::Configuration.new + end + def environment @environment ||= Spree::Core::Environment.new(self).tap do |env| env.calculators.promotion_actions_create_adjustments = %w[ diff --git a/core/spec/models/spree/user_return_to_storer/rules/authentication_rule_spec.rb b/core/spec/models/spree/user_return_to_storer/rules/authentication_rule_spec.rb new file mode 100644 index 00000000000..c4274c8dcb7 --- /dev/null +++ b/core/spec/models/spree/user_return_to_storer/rules/authentication_rule_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Spree::UserReturnToStorer::Rules::AuthenticationRule do + describe '#match?' do + let(:login_path) { '/sign_in' } + let(:request) { double(fullpath: fullpath) } + let(:controller) do + double( + request: request, + spree_login_path: login_path + ) + end + + subject { described_class.match?(controller) } + + context 'when the request full path is an authentication route' do + let!(:fullpath) { login_path } + + it { is_expected.to be true } + end + + context 'when the request full path is not an authentication route' do + let!(:fullpath) { '/products/baseball-cap' } + + it { is_expected.to be false } + end + end +end diff --git a/core/spec/models/spree/user_return_to_storer_spec.rb b/core/spec/models/spree/user_return_to_storer_spec.rb new file mode 100644 index 00000000000..0748bea401e --- /dev/null +++ b/core/spec/models/spree/user_return_to_storer_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Spree::UserReturnToStorer do + subject { described_class.new(controller) } + + let(:fullpath) { '/products/baseball-cap' } + let(:session) { {} } + let(:request) { double(fullpath: fullpath) } + let(:controller) do + instance_double( + ApplicationController, + request: request, + session: session + ) + end + + module CustomRule + def self.match?(_controller) + true + end + end + + after :each do + described_class.rules.delete('CustomRule') + end + + describe '::rules' do + it 'includes default rules' do + rule = Spree::UserReturnToStorer::Rules::AuthenticationRule + expect(described_class.rules).to include(rule) + end + + it 'can add new rules' do + described_class.rules << CustomRule + expect(described_class.rules).to include(CustomRule) + end + end + + describe '#store_location' do + context 'when at least one rule matches' do + it 'does not set the path value into the session' do + described_class.rules << CustomRule + subject.store_location + expect(session[:spree_user_return_to]).to be_nil + end + end + + context 'when no rule matches' do + it 'sets the path value into the session' do + described_class.rules << CustomRule + described_class.rules.delete('CustomRule') + subject.store_location + expect(session[:spree_user_return_to]).to eql fullpath + end + end + end +end