Merge pull request #1041 from charlie-wasp/fix/get-request-warning

Warn only on GET requests for a login path

lib/omniauth/strategy.rb

@@ -180,9 +180,10 @@ module OmniAuth
         raise(error)
       end
 
-      warn_if_using_get
-
       @env = env
+
+      warn_if_using_get_on_request_path
+
       @env['omniauth.strategy'] = self if on_auth_path?
 
       return mock_call!(env) if OmniAuth.config.test_mode
@@ -201,7 +202,8 @@ module OmniAuth
       @app.call(env)
     end
 
-    def warn_if_using_get
+    def warn_if_using_get_on_request_path
+      return unless on_request_path?
       return unless OmniAuth.config.allowed_request_methods.include?(:get)
       return if OmniAuth.config.silence_get_warning
 

spec/omniauth/strategy_spec.rb

@@ -986,6 +986,9 @@ describe OmniAuth::Strategy do
       end
 
       context 'with allowed GET' do
+        let(:path) { '/auth/test' }
+        let(:get_env) { make_env(path, 'REQUEST_METHOD' => 'GET') }
+
         before(:context) do
           @old_allowed_request_methods = OmniAuth.config.allowed_request_methods
           OmniAuth.config.allowed_request_methods = %i[post get]
@@ -994,10 +997,27 @@ describe OmniAuth::Strategy do
         it 'allows a request without authenticity token' do
           expect(strategy).to receive(:fail!).with('Request Phase', kind_of(StandardError))
 
-          get_env = make_env('/auth/test', 'REQUEST_METHOD' => 'GET')
           strategy.call(get_env)
         end
 
+        describe 'warning message logging' do
+          before { allow(strategy).to receive(:log) }
+
+          it 'logs warning message' do
+            strategy.call(get_env)
+            expect(strategy).to have_received(:log).with(:warn, a_string_matching('You are using GET as an allowed request method')).once
+          end
+
+          context 'when not login path is requested' do
+            let(:path) { '/example/path' }
+
+            it 'does not log warning message' do
+              strategy.call(get_env)
+              expect(strategy).not_to have_received(:log).with(:warn, a_string_matching('You are using GET as an allowed request method'))
+            end
+          end
+        end
+
         after(:context) do
           OmniAuth.config.allowed_request_methods = @old_allowed_request_methods
         end