diff --git a/app/controllers/shopify_app/sessions_controller.rb b/app/controllers/shopify_app/sessions_controller.rb index 75cf7c205..3a6732e73 100644 --- a/app/controllers/shopify_app/sessions_controller.rb +++ b/app/controllers/shopify_app/sessions_controller.rb @@ -125,7 +125,7 @@ def validate_shop_presence end def copy_return_to_param_to_session - session[:return_to] = params[:return_to] if params[:return_to] + session[:return_to] = RedirectSafely.make_safe(params[:return_to], '/') if params[:return_to] end def render_invalid_shop_error diff --git a/lib/shopify_app.rb b/lib/shopify_app.rb index 9bd1d8262..3797c7d61 100644 --- a/lib/shopify_app.rb +++ b/lib/shopify_app.rb @@ -4,6 +4,7 @@ # deps require 'shopify_api' require 'omniauth-shopify-oauth2' +require 'redirect_safely' module ShopifyApp def self.rails6? diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index 4b9c2a4f7..f0677ae6a 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -139,7 +139,7 @@ def login_url_params(top_level:) query_params = {} query_params[:shop] = sanitized_params[:shop] if params[:shop].present? - return_to = session[:return_to] || params[:return_to] + return_to = RedirectSafely.make_safe(session[:return_to] || params[:return_to], nil) if return_to.present? && return_to_param_required? query_params[:return_to] = return_to diff --git a/shopify_app.gemspec b/shopify_app.gemspec index e97749d8c..300a0df93 100644 --- a/shopify_app.gemspec +++ b/shopify_app.gemspec @@ -18,6 +18,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency('shopify_api', '~> 9.0.2') s.add_runtime_dependency('omniauth-shopify-oauth2', '~> 2.2.2') s.add_runtime_dependency('jwt', '~> 2.2.1') + s.add_runtime_dependency('redirect_safely', '~> 1.0') s.add_development_dependency('rake') s.add_development_dependency('byebug') diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 6e927eadf..8f8a7de33 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -83,6 +83,16 @@ class SessionsControllerTest < ActionController::TestCase assert_redirected_to_top_level(shopify_domain) end + test '#new stores root path when return_to url is absolute' do + get :new, params: { shop: 'my-shop', return_to: '//example.com' } + assert_equal '/', session[:return_to] + end + + test '#new stores only relative path for return_to in the session' do + get :new, params: { shop: 'my-shop', return_to: '/page' } + assert_equal '/page', session[:return_to] + end + test '#new sets the top_level_oauth cookie if a valid shop param exists and user agent supports cookie partitioning' do request.env['HTTP_USER_AGENT'] = 'Version/12.0 Safari' get :new, params: { shop: 'my-shop' }