From 93a37b761305ca1b7a120287b6cd89cad3dace28 Mon Sep 17 00:00:00 2001
From: Bryan Talbot <bryan.talbot@ijji.com>
Date: Sat, 15 Jun 2024 16:17:40 -0700
Subject: [PATCH 1/2] Fix: handling of SCRIPT_NAME for registration_path

The registration_path must include the SCRIPT_NAME when it exists to allow requests to
be seen as properly `on_path?` and to be routed to the correct rack app. Additionally,
the private method `#registration_result` must not replace the PATH_INFO with `#callback_path`
as `OmniAuth::Strategy#callback_path` already includes the SCRIPT_PATH leading to the SCRIPT_PATH
portion being duplicated when registration completes and is forwarded to the callback endpoint.

Spec tests added to test all combinations of default and optional values of SCRIPT_PATH,
OmniAuth::Strategy.path_prefix, and Identity.name.
---
 lib/omniauth/strategies/identity.rb       |  4 +-
 spec/omniauth/strategies/identity_spec.rb | 60 ++++++++++++++++++-----
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/lib/omniauth/strategies/identity.rb b/lib/omniauth/strategies/identity.rb
index c6ea175..f0483b1 100644
--- a/lib/omniauth/strategies/identity.rb
+++ b/lib/omniauth/strategies/identity.rb
@@ -96,7 +96,7 @@ def registration_phase
       info { identity.info }
 
       def registration_path
-        options[:registration_path] || "#{path_prefix}/#{name}/register"
+        options[:registration_path] || "#{script_name}#{path_prefix}/#{name}/register"
       end
 
       def on_registration_path?
@@ -169,7 +169,7 @@ def registration_failure(message)
 
       def registration_result
         if @identity.persisted?
-          env['PATH_INFO'] = callback_path
+          env['PATH_INFO'] = "#{path_prefix}/#{name}/callback"
           callback_phase
         else
           registration_failure(options[:registration_failure_message])
diff --git a/spec/omniauth/strategies/identity_spec.rb b/spec/omniauth/strategies/identity_spec.rb
index 46cd709..07d5a7b 100644
--- a/spec/omniauth/strategies/identity_spec.rb
+++ b/spec/omniauth/strategies/identity_spec.rb
@@ -11,6 +11,7 @@
   let(:auth_hash) { env_hash['omniauth.auth'] }
   let(:identity_hash) { env_hash['omniauth.identity'] }
   let(:identity_options) { {} }
+  let(:app_options) { {} }
   let(:anon_ar) do
     AnonymousActiveRecord.generate(
       parent_klass: 'OmniAuth::Identity::Models::ActiveRecord',
@@ -24,25 +25,58 @@ def balloon
     end
   end
 
-  # customize rack app for testing, if block is given, reverts to default
-  # rack app after testing is done
-  def set_app!(identity_options = {})
-    old_app = app
+  # the rack testing framework will call this method (to get app) as needed to run tests
+  def app
+    opts = identity_options.reverse_merge({ model: anon_ar })
+    script_name = app_options[:script_name]
     self.app = Rack::Builder.app do
       use Rack::Session::Cookie, secret: '1234567890qwertyuiop'
-      use OmniAuth::Strategies::Identity, identity_options
+      if script_name
+        map script_name do
+          use OmniAuth::Strategies::Identity, opts
+        end
+      else
+        use OmniAuth::Strategies::Identity, opts
+      end
       run ->(env) { [404, { 'env' => env }, ['HELLO!']] }
     end
-    if block_given?
-      yield
-      self.app = old_app
-    end
-    app
   end
 
-  before do
-    opts = identity_options.reverse_merge({ model: anon_ar })
-    set_app!(opts)
+  describe 'path handling' do
+    script_names = [nil, '/my_path']
+    path_prefixes = ['/auth', '/my_auth', '']
+    provider_names = ['identity', 'my_id']
+    script_names.product(path_prefixes, provider_names).each do |script_name, path_prefix, provider_name|
+      ext_base_path = "#{script_name}#{path_prefix}/#{provider_name}"
+      ext_callback_path = "#{ext_base_path}/callback"
+      ext_register_path = "#{ext_base_path}/register"
+
+      context "with base path '#{ext_base_path}'" do
+        let(:app_options) { { script_name: script_name } }
+        let(:identity_options) {
+          { enable_registration: true, enable_login: true, path_prefix: path_prefix, name: provider_name }
+        }
+        it "calls app" do
+          get "#{script_name}/hello/world"
+          expect(last_response.body).to eq('HELLO!')
+        end
+        it '#request_phase displays form' do
+          get ext_base_path
+          expect(last_response.body).not_to eq('HELLO!')
+          expect(last_response.body).to include('<form')
+          expect(last_response.body).to include("action='#{ext_callback_path}'")
+          expect(last_response.body).to include("href='#{ext_register_path}'")
+        end
+        it '#callback_phase is handled' do
+          get ext_callback_path
+          expect(last_response.body).not_to eq('HELLO!')
+        end
+        it '#registration_phase is handled' do
+          get ext_register_path
+          expect(last_response.body).not_to eq('HELLO!')
+        end
+      end
+    end
   end
 
   describe '#request_phase' do

From bf769276b4696141bfd195ee9a71f9c30d1c1452 Mon Sep 17 00:00:00 2001
From: Boban Sugareski <boban@actionvfx.com>
Date: Wed, 18 Sep 2024 10:33:13 +0200
Subject: [PATCH 2/2] Improve readability in #identity method of
 OmniAuth::Strategies::Identity

---
 lib/omniauth/strategies/identity.rb | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/lib/omniauth/strategies/identity.rb b/lib/omniauth/strategies/identity.rb
index c6ea175..5113c14 100644
--- a/lib/omniauth/strategies/identity.rb
+++ b/lib/omniauth/strategies/identity.rb
@@ -104,12 +104,9 @@ def on_registration_path?
       end
 
       def identity
-        if options[:locate_conditions].is_a? Proc
-          conditions = instance_exec(request, &options[:locate_conditions])
-          conditions.to_hash
-        else
-          conditions = options[:locate_conditions].to_hash
-        end
+        conditions = options[:locate_conditions]
+        conditions = conditions.is_a?(Proc) ? instance_exec(request, &conditions).to_hash : conditions.to_hash
+
         @identity ||= model.authenticate(conditions, request['password'])
       end