From 8e35686783215c9acd4585676f880ef8bfc36a9e Mon Sep 17 00:00:00 2001 From: atolix Date: Wed, 1 May 2024 15:00:19 +0900 Subject: [PATCH 1/6] Add an option 'use_redirect_back_or_to_by_rails' to avoid definition conflicts 'redirect_back_or_to' Fix: https://github.com/Sorcery/sorcery/issues/296 Rails 7 released a new method called `redirect_back_or_to` as a replacement for `redirect_back`. That may conflicts with the method by the same name defined by Sorcery. This commit adds an option to set whether to use `redirect_back_or_to` defined by Rails 7, and add the method `redirect_to_before_login_path` as an alternative to sorcery's `redirect_back_or_to. ref: https://github.com/rails/rails/pull/40671 --- .../sorcery/templates/initializer.rb | 9 +++++ lib/sorcery/controller.rb | 11 +++++- lib/sorcery/controller/config.rb | 6 +++- spec/controllers/controller_spec.rb | 35 +++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/lib/generators/sorcery/templates/initializer.rb b/lib/generators/sorcery/templates/initializer.rb index 5cec7018..c5185b5f 100644 --- a/lib/generators/sorcery/templates/initializer.rb +++ b/lib/generators/sorcery/templates/initializer.rb @@ -21,6 +21,15 @@ # # config.save_return_to_url = + # Set whether to use 'redirect_back_or_to' defined in Rails 7. + # Rails 7 released a new method called 'redirect_back_or_to' as a replacement for 'redirect_back'. + # That may conflict with the method by the same name defined by Sorcery. + # If you set this option to true, Sorcery's 'redirect_back_or_to' calls 'super' to use + # the method of the same name defined in Rails 7. + # Default: `false` + # + # config.use_redirect_back_or_to_by_rails = + # Set domain option for cookies; Useful for remember_me submodule. # Default: `nil` # diff --git a/lib/sorcery/controller.rb b/lib/sorcery/controller.rb index cd8e4284..f7cce41c 100644 --- a/lib/sorcery/controller.rb +++ b/lib/sorcery/controller.rb @@ -96,7 +96,16 @@ def current_user=(user) # used when a user tries to access a page while logged out, is asked to login, # and we want to return him back to the page he originally wanted. - def redirect_back_or_to(url, flash_hash = {}) + def redirect_back_or_to(...) + if Config.use_redirect_back_or_to_by_rails + super + else + warn('[WARNING] `redirect_back_or_to` overrides the method of the same name defined in Rails 7. If you want to avoid overriding, you can set `config.use_redirect_back_or_to_by_rails = true` and use `redirect_to_before_login_path`.') + redirect_to_before_login_path(...) + end + end + + def redirect_to_before_login_path(url, flash_hash = {}) redirect_to(session[:return_to_url] || url, flash: flash_hash) session[:return_to_url] = nil end diff --git a/lib/sorcery/controller/config.rb b/lib/sorcery/controller/config.rb index 1a9bf112..1f04a91d 100644 --- a/lib/sorcery/controller/config.rb +++ b/lib/sorcery/controller/config.rb @@ -20,6 +20,9 @@ class << self attr_accessor :after_logout attr_accessor :after_remember_me + # set whether to use 'redirect_back_or_to' defined in Rails 7. + attr_accessor :use_redirect_back_or_to_by_rails + def init! @defaults = { :@user_class => nil, @@ -32,7 +35,8 @@ def init! :@after_logout => Set.new, :@after_remember_me => Set.new, :@save_return_to_url => true, - :@cookie_domain => nil + :@cookie_domain => nil, + :@use_redirect_back_or_to_by_rails => false } end diff --git a/spec/controllers/controller_spec.rb b/spec/controllers/controller_spec.rb index dd041ac0..4465186a 100644 --- a/spec/controllers/controller_spec.rb +++ b/spec/controllers/controller_spec.rb @@ -22,6 +22,12 @@ expect(Sorcery::Controller::Config.not_authenticated_action).to eq :my_action end + + it "enables configuration option 'use_redirect_back_or_to_by_rails'" do + sorcery_controller_property_set(:use_redirect_back_or_to_by_rails, true) + + expect(Sorcery::Controller::Config.use_redirect_back_or_to_by_rails).to be true + end end # ----------------- PLUGIN ACTIVATED ----------------------- @@ -186,5 +192,34 @@ expect(assigns[:result]).to eq user end + + describe 'redirect_back_or_to' do + describe 'use_redirect_back_or_to_by_rails' do + context 'when true' do + before do + sorcery_controller_property_set(:use_redirect_back_or_to_by_rails, true) + allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return('http://test.host/referer_action') + end + + it 'uses Rails 7 redirect_back_or_to method' do + get :test_return_to + + expect(response).to redirect_to('http://test.host/referer_action') + end + end + + context 'when false' do + before { sorcery_controller_property_set(:use_redirect_back_or_to_by_rails, false) } + + it 'uses Sorcery redirect_back_or_to method' do + session[:return_to_url] = 'http://test.host/some_action' + get :test_return_to + + expect(response).to redirect_to('http://test.host/some_action') + expect(flash[:notice]).to eq 'haha!' + end + end + end + end end end From 03c68940f9a25f7a6250895807f5f77f5879b2ec Mon Sep 17 00:00:00 2001 From: atolix <82761106+atolix@users.noreply.github.com> Date: Fri, 10 May 2024 00:52:10 +0900 Subject: [PATCH 2/6] add warning message that config.use_redirect_back_or_to_by_rails = true will become the default --- lib/sorcery/controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sorcery/controller.rb b/lib/sorcery/controller.rb index f7cce41c..c9451720 100644 --- a/lib/sorcery/controller.rb +++ b/lib/sorcery/controller.rb @@ -100,7 +100,7 @@ def redirect_back_or_to(...) if Config.use_redirect_back_or_to_by_rails super else - warn('[WARNING] `redirect_back_or_to` overrides the method of the same name defined in Rails 7. If you want to avoid overriding, you can set `config.use_redirect_back_or_to_by_rails = true` and use `redirect_to_before_login_path`.') + warn('[WARNING] `redirect_back_or_to` overrides the method of the same name defined in Rails 7. If you want to avoid overriding, you can set `config.use_redirect_back_or_to_by_rails = true` and use `redirect_to_before_login_path`. In a future version, `config.use_redirect_back_or_to_by_rails = true` will become the default.') redirect_to_before_login_path(...) end end From 0862950d758dbbe68c49c3f4048ca9ce8834364f Mon Sep 17 00:00:00 2001 From: atolix <82761106+atolix@users.noreply.github.com> Date: Fri, 10 May 2024 01:20:33 +0900 Subject: [PATCH 3/6] add example 'when Rails::VERSION::MAJOR < 7' --- spec/controllers/controller_spec.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/spec/controllers/controller_spec.rb b/spec/controllers/controller_spec.rb index 4465186a..44d3dd16 100644 --- a/spec/controllers/controller_spec.rb +++ b/spec/controllers/controller_spec.rb @@ -201,10 +201,18 @@ allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return('http://test.host/referer_action') end - it 'uses Rails 7 redirect_back_or_to method' do - get :test_return_to + context 'when Rails::VERSION::MAJOR >= 7', skip: Rails::VERSION::MAJOR < 7 do + it 'uses Rails 7 redirect_back_or_to method' do + get :test_return_to + + expect(response).to redirect_to('http://test.host/referer_action') + end + end - expect(response).to redirect_to('http://test.host/referer_action') + context 'when Rails::VERSION::MAJOR < 7', skip: Rails::VERSION::MAJOR >= 7 do + it 'raise NoMethodError' do + expect { get :test_return_to }.to raise_error(NoMethodError) + end end end From d78f5fed1eec4fbd66abd4d36f986299dcd9d234 Mon Sep 17 00:00:00 2001 From: atolix <82761106+atolix@users.noreply.github.com> Date: Tue, 9 Sep 2025 22:57:45 +0900 Subject: [PATCH 4/6] Update comment in initializer to use `redirect_to_before_login_path` --- lib/generators/sorcery/templates/initializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/generators/sorcery/templates/initializer.rb b/lib/generators/sorcery/templates/initializer.rb index c5185b5f..af1f7f26 100644 --- a/lib/generators/sorcery/templates/initializer.rb +++ b/lib/generators/sorcery/templates/initializer.rb @@ -16,7 +16,7 @@ # config.not_authenticated_action = # When a non logged-in user tries to enter a page that requires login, save - # the URL he wants to reach, and send him there after login, using 'redirect_back_or_to'. + # the URL he wants to reach, and send him there after login, using 'redirect_to_before_login_path'. # Default: `true` # # config.save_return_to_url = From 3077b41590fc529bf1c2fc2c65ef00041c399883 Mon Sep 17 00:00:00 2001 From: atolix <82761106+atolix@users.noreply.github.com> Date: Tue, 9 Sep 2025 23:18:57 +0900 Subject: [PATCH 5/6] fixed tests to use `redirect_to_before_login_path` Replaced all existing `redirect_back_or_to` calls with `redirect_to_before_login_path`, and added a dedicated method for testing the updated behavior of `redirect_back_or_to` --- spec/controllers/controller_spec.rb | 6 +-- .../app/controllers/sorcery_controller.rb | 39 +++++++++++-------- spec/rails_app/config/routes.rb | 1 + 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/spec/controllers/controller_spec.rb b/spec/controllers/controller_spec.rb index 44d3dd16..bb1ced2f 100644 --- a/spec/controllers/controller_spec.rb +++ b/spec/controllers/controller_spec.rb @@ -203,7 +203,7 @@ context 'when Rails::VERSION::MAJOR >= 7', skip: Rails::VERSION::MAJOR < 7 do it 'uses Rails 7 redirect_back_or_to method' do - get :test_return_to + get :test_redirect_back_or_to expect(response).to redirect_to('http://test.host/referer_action') end @@ -211,7 +211,7 @@ context 'when Rails::VERSION::MAJOR < 7', skip: Rails::VERSION::MAJOR >= 7 do it 'raise NoMethodError' do - expect { get :test_return_to }.to raise_error(NoMethodError) + expect { get :test_redirect_back_or_to }.to raise_error(NoMethodError) end end end @@ -221,7 +221,7 @@ it 'uses Sorcery redirect_back_or_to method' do session[:return_to_url] = 'http://test.host/some_action' - get :test_return_to + get :test_redirect_back_or_to expect(response).to redirect_to('http://test.host/some_action') expect(flash[:notice]).to eq 'haha!' diff --git a/spec/rails_app/app/controllers/sorcery_controller.rb b/spec/rails_app/app/controllers/sorcery_controller.rb index 8214e36a..2c136eef 100644 --- a/spec/rails_app/app/controllers/sorcery_controller.rb +++ b/spec/rails_app/app/controllers/sorcery_controller.rb @@ -36,6 +36,11 @@ def test_auto_login end def test_return_to + @user = login(params[:email], params[:password]) + redirect_to_before_login_path(:index, notice: 'haha!') + end + + def test_redirect_back_or_to @user = login(params[:email], params[:password]) redirect_back_or_to(:index, notice: 'haha!') end @@ -314,7 +319,7 @@ def test_login_from_battlenet def test_return_to_with_external_twitter if (@user = login_from(:twitter)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -322,7 +327,7 @@ def test_return_to_with_external_twitter def test_return_to_with_external_jira if (@user = login_from(:jira)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -332,7 +337,7 @@ def test_return_to_with_external_jira def test_return_to_with_external_facebook if (@user = login_from(:facebook)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -340,7 +345,7 @@ def test_return_to_with_external_facebook def test_return_to_with_external_github if (@user = login_from(:github)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -348,7 +353,7 @@ def test_return_to_with_external_github def test_return_to_with_external_paypal if (@user = login_from(:paypal)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -356,7 +361,7 @@ def test_return_to_with_external_paypal def test_return_to_with_external_wechat if (@user = login_from(:wechat)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -364,7 +369,7 @@ def test_return_to_with_external_wechat def test_return_to_with_external_microsoft if (@user = login_from(:microsoft)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -372,7 +377,7 @@ def test_return_to_with_external_microsoft def test_return_to_with_external_google if (@user = login_from(:google)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -380,7 +385,7 @@ def test_return_to_with_external_google def test_return_to_with_external_liveid if (@user = login_from(:liveid)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -388,7 +393,7 @@ def test_return_to_with_external_liveid def test_return_to_with_external_vk if (@user = login_from(:vk)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -396,7 +401,7 @@ def test_return_to_with_external_vk def test_return_to_with_external_salesforce if (@user = login_from(:salesforce)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -404,7 +409,7 @@ def test_return_to_with_external_salesforce def test_return_to_with_external_slack if (@user = login_from(:slack)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -412,7 +417,7 @@ def test_return_to_with_external_slack def test_return_to_with_external_instagram if (@user = login_from(:instagram)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -420,7 +425,7 @@ def test_return_to_with_external_instagram def test_return_to_with_external_auth0 if (@user = login_from(:auth0)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -428,7 +433,7 @@ def test_return_to_with_external_auth0 def test_return_to_with_external_line if @user = login_from(:line) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -436,7 +441,7 @@ def test_return_to_with_external_line def test_return_to_with_external_discord if (@user = login_from(:discord)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end @@ -444,7 +449,7 @@ def test_return_to_with_external_discord def test_return_to_with_external_battlenet if (@user = login_from(:battlenet)) - redirect_back_or_to 'bla', notice: 'Success!' + redirect_to_before_login_path 'bla', notice: 'Success!' else redirect_to 'blu', alert: 'Failed!' end diff --git a/spec/rails_app/config/routes.rb b/spec/rails_app/config/routes.rb index 4bf82c29..c7b80d2a 100644 --- a/spec/rails_app/config/routes.rb +++ b/spec/rails_app/config/routes.rb @@ -6,6 +6,7 @@ get :test_logout get :some_action post :test_return_to + post :test_redirect_back_or_to get :test_auto_login post :test_login_with_remember_in_login get :test_login_from_cookie From 377f2ed2194e44bd3bc4a088563b9dd71b8733e8 Mon Sep 17 00:00:00 2001 From: atolix <82761106+atolix@users.noreply.github.com> Date: Tue, 30 Sep 2025 21:36:44 +0900 Subject: [PATCH 6/6] Delete unnecessary Rails:::Version < 7 test --- spec/controllers/controller_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/controllers/controller_spec.rb b/spec/controllers/controller_spec.rb index bb1ced2f..8a8abccf 100644 --- a/spec/controllers/controller_spec.rb +++ b/spec/controllers/controller_spec.rb @@ -208,12 +208,6 @@ expect(response).to redirect_to('http://test.host/referer_action') end end - - context 'when Rails::VERSION::MAJOR < 7', skip: Rails::VERSION::MAJOR >= 7 do - it 'raise NoMethodError' do - expect { get :test_redirect_back_or_to }.to raise_error(NoMethodError) - end - end end context 'when false' do