From 7a336a71a80e3b0a447a522f6ef2b66965b0457e Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 11 Oct 2019 17:20:35 +0200 Subject: [PATCH 1/3] Add UserLastUrlStorer service object Spree::UserLastUrlStorer 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 --- core/app/models/spree/user_last_url_storer.rb | 46 ++++++++++++++ .../rules/authentication_rule.rb | 36 +++++++++++ core/lib/spree/app_configuration.rb | 6 ++ .../rules/authentication_rule_spec.rb | 31 ++++++++++ .../models/spree/user_last_url_storer_spec.rb | 60 +++++++++++++++++++ 5 files changed, 179 insertions(+) create mode 100644 core/app/models/spree/user_last_url_storer.rb create mode 100644 core/app/models/spree/user_last_url_storer/rules/authentication_rule.rb create mode 100644 core/spec/models/spree/user_last_url_storer/rules/authentication_rule_spec.rb create mode 100644 core/spec/models/spree/user_last_url_storer_spec.rb diff --git a/core/app/models/spree/user_last_url_storer.rb b/core/app/models/spree/user_last_url_storer.rb new file mode 100644 index 00000000000..01cbb1bd37c --- /dev/null +++ b/core/app/models/spree/user_last_url_storer.rb @@ -0,0 +1,46 @@ +# 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 UserLastUrlStorer + # 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::UserLastUrlStorer.rules << 'CustomRule' + # + # @example it can be used also for removing unwanted rules + # Spree::UserLastUrlStorer.rules.delete('CustomRule') + # + def self.rules + Spree::Config.user_last_url_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_last_url_storer/rules/authentication_rule.rb b/core/app/models/spree/user_last_url_storer/rules/authentication_rule.rb new file mode 100644 index 00000000000..80bf735ebc6 --- /dev/null +++ b/core/app/models/spree/user_last_url_storer/rules/authentication_rule.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Spree + class UserLastUrlStorer + 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 ||= {} + @disallowed_urls[controller.controller_name] ||= 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 f6128e1bf0e..84fa3d2f460 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -474,6 +474,12 @@ def events @events_configuration ||= Spree::Event::Configuration.new end + def user_last_url_storer_rules + @user_last_url_storer_rules ||= ::Spree::Core::ClassConstantizer::Set.new.tap do |set| + set << 'Spree::UserLastUrlStorer::Rules::AuthenticationRule' + end + 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_last_url_storer/rules/authentication_rule_spec.rb b/core/spec/models/spree/user_last_url_storer/rules/authentication_rule_spec.rb new file mode 100644 index 00000000000..121ab331139 --- /dev/null +++ b/core/spec/models/spree/user_last_url_storer/rules/authentication_rule_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Spree::UserLastUrlStorer::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, + controller_name: 'controller_double' + ) + 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_last_url_storer_spec.rb b/core/spec/models/spree/user_last_url_storer_spec.rb new file mode 100644 index 00000000000..aa3c7794583 --- /dev/null +++ b/core/spec/models/spree/user_last_url_storer_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Spree::UserLastUrlStorer 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, + controller_name: 'app_controller_double' + ) + 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::UserLastUrlStorer::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 From 94d897a6ef38e6c0cce86d4f882c7be0cd76973a Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 11 Oct 2019 17:26:01 +0200 Subject: [PATCH 2/3] Use UserLastUrlStorer in ControllerHelpers::Auth module By using Spree::UserLastUrlStorer one can change ControllerHelpers::Auth#store_location behavior without resorting to monkey patching. --- .../templates/config/initializers/spree.rb.tt | 8 ++++++++ core/lib/spree/core/controller_helpers/auth.rb | 14 +------------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt b/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt index 04175bda306..7c38954a388 100644 --- a/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt +++ b/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt @@ -93,3 +93,11 @@ Spree.user_class = <%= (options[:user_class].blank? ? "Spree::LegacyUser" : opti # just uncomment the following code and change it as you need. # # Spree::Model.whitelisted_ransackable_attributes << 'field' + +# Rules for avoiding to store the current path into session for redirects +# When at least one rule is matched, the request path will not be stored +# in session. +# You can add your custom rules by uncommenting this line and changing +# the class name: +# +# Spree::UserLastUrlStorer.rules << 'Spree::UserLastUrlStorer::Rules::AuthenticationRule' diff --git a/core/lib/spree/core/controller_helpers/auth.rb b/core/lib/spree/core/controller_helpers/auth.rb index 5f2bec80cc8..2bcb6115e15 100644 --- a/core/lib/spree/core/controller_helpers/auth.rb +++ b/core/lib/spree/core/controller_helpers/auth.rb @@ -50,19 +50,7 @@ def set_guest_token end def store_location - # disallow return to login, logout, signup pages - authentication_routes = [:spree_signup_path, :spree_login_path, :spree_logout_path] - disallowed_urls = [] - authentication_routes.each do |route| - if respond_to?(route) - disallowed_urls << send(route) - end - end - - disallowed_urls.map!{ |url| url[/\/\w+$/] } - unless disallowed_urls.include?(request.fullpath) - session['spree_user_return_to'] = request.fullpath.gsub('//', '/') - end + Spree::UserLastUrlStorer.new(self).store_location end # proxy method to *possible* spree_current_user method From 55d17721dd566e33a8b8f3d95a045406fdc982c2 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 18 Oct 2019 10:05:38 +0200 Subject: [PATCH 3/3] Add documentation for after login redirects --- .../customizing-after-login-redirects.html.md | 56 +++++++++++++++++++ .../customizations/overview.html.md | 6 +- 2 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 guides/source/developers/customizations/customizing-after-login-redirects.html.md diff --git a/guides/source/developers/customizations/customizing-after-login-redirects.html.md b/guides/source/developers/customizations/customizing-after-login-redirects.html.md new file mode 100644 index 00000000000..b70d5b10754 --- /dev/null +++ b/guides/source/developers/customizations/customizing-after-login-redirects.html.md @@ -0,0 +1,56 @@ +# Customizing After-login Redirects + +Standard Solidus installations use the `solidus_auth_devise` gem +in order to provide user authentication. The gem is based on +`Devise`, a very successful authentication gem for Rails. + +When the unauthenticated user visits an authentication-protected page, they're +first redirected to the login page, eventually after successful login they're +redirected back to the page they were originally wanting to visit. + +Before redirecting the user to the login page, Solidus stores the original URL +that the user wanted to visit into Rails application session cookie, ie. +`session[:spree_return_to]`. + +There are some URLs that we need to avoid storing in session, othwewise +inifite-loops would occur after successful authentication. + +All of these URLs with a standard Solidus installation are related to the +authentication process, but you may need to add more, for example because you +added some more authentication URLs. + +Solidus uses rules managed by the service object [`Spree::UserLastUrlStorer`][user-last-url-storer] +in order to decide whether the current path should be stored or not. The +default rule is defined in [`Spree::UserLastUrlStorer::Rules::AuthenticationRule`][auth-rule]. + +In order to add your custom behavior, you can create a new rule: + +```ruby +module Spree + class UserLastUrlStorer + module Rules + module FacebookLoginRule + extend self + + def match?(controller) + controller.controller_name == "sessions" && + action_name == "facebook_login" + end + end + end + end +end +``` + +After that, you need to register your new rule module, for example by adding +this line in `config/spree.rb` file: + +```ruby +Spree::UserLastUrlStorer.rules << 'Spree::UserLastUrlStorer::Rules::FacebookLoginRule' +``` + +Please note that, when at least one rule is met (`#match?` returns `true`) then +the current path **is not** stored in the session. + +[user-last-url-storer]: https://github.com/solidusio/solidus/blob/master/core/app/models/spree/user_last_url_storer.rb +[auth-rule]: https://github.com/solidusio/solidus/blob/master/core/app/models/spree/user_last_url_storer/rules/authentication_rule.rb diff --git a/guides/source/developers/customizations/overview.html.md b/guides/source/developers/customizations/overview.html.md index ead8c47b918..96e67316d33 100644 --- a/guides/source/developers/customizations/overview.html.md +++ b/guides/source/developers/customizations/overview.html.md @@ -4,8 +4,8 @@ Solidus is very flexible. It allows you to customize every part of the store, bo the customer-facing storefront part (also called "frontend") and the admin panel (also called "backend"). -Some customizations are very easy to implement, even for inexperienced developers. -Others may require a solid understanding of the Ruby and Ruby on Rails ecosystem, +Some customizations are very easy to implement, even for inexperienced developers. +Others may require a solid understanding of the Ruby and Ruby on Rails ecosystem, which are the language and the framework that power Solidus, respectively. This guide will provide a brief introduction to the different types of customization that are possible using Solidus. @@ -19,6 +19,7 @@ that are possible using Solidus. - [Learn How to Customize Permissions][permissions] - [Learn How to Customize Attributes][attributes] - [Learn How to Customize Mailers][mailers] +- [Learn How to Customize After-login Redirects][after-login-redirects] [storefront]: customizing-storefront.html [admin]: customizing-admin.html @@ -27,3 +28,4 @@ that are possible using Solidus. [permissions]: customizing-permissions.html [attributes]: customizing-attributes.html [mailers]: customizing-mailers.html +[after-login-redirects]: customizing-after-login-redirects.html