How not to send the `state` parameter? about omniauth_openid_connect HOT 21 OPEN

Yeah, I'm trying to avoid adding yet another config parameter here, plus clear up some confusion about how require_state is supposed to work. I'm not sure how require_state ever worked as false because stored_state will always have a value since new_state always sets the value to some random value.

from omniauth_openid_connect.

@skycocker @syakovyn @bufferoverflow I think require_state was added in #127. Do you see any reason why we couldn't simplify this via:

diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb
index ebfaaa1..6d80a66 100644
--- a/lib/omniauth/strategies/openid_connect.rb
+++ b/lib/omniauth/strategies/openid_connect.rb
@@ -120,7 +120,12 @@ module OmniAuth
       def callback_phase
         error = params['error_reason'] || params['error']
         error_description = params['error_description'] || params['error_reason']
-        invalid_state = (options.require_state && params['state'].to_s.empty?) || params['state'] != stored_state
+        invalid_state =
+          if options.require_state
+            params['state'].to_s.empty? || params['state'] != stored_state
+          else
+            false
+          end
 
         raise CallbackError, error: params['error'], reason: error_description, uri: params['error_uri'] if error
         raise CallbackError, error: :csrf_detected, reason: "Invalid 'state' parameter" if invalid_state

I'm not sure if state is being used somewhere else, but if it's just forwarded back, there's no use in generating it?

from omniauth_openid_connect.

Oh, I see that was proposed in #127 (comment).

The issue here is that if the state is generated by the authorize_uri, chances are the OpenID provider will pass it along. So maybe we should have gone with the original implementation?

from omniauth_openid_connect.

Hi! Thanks for looking into this.

In the mean time, I've gone ahead and made a working though naive implementation of a solution introducing an additional send_state config parameter.

I went with this solution because I didn't want to alter the behavior of the require_state config parameter.

amessinger@2a2510c

Hope this helps!

from omniauth_openid_connect.