Make the state parameter verification optional

Author: skycocker

CHANGELOG.md

@@ -1,3 +1,7 @@
+# v0.4.1 (06.02.2022)
+
+- Make the state parameter verification optional [#122](https://github.com/omniauth/omniauth_openid_connect/pull/122)
+
 # v0.4.0 (06.02.2022)
 
 - Support dynamic parameters to the authorize URI [#90](https://github.com/omniauth/omniauth_openid_connect/pull/90)

README.md

@@ -49,12 +49,13 @@ config.omniauth :openid_connect, {
 
 | Field                        | Description                                                                                                                                                   | Required | Default                    | Example/Options                                     |
 |------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|----------------------------|-----------------------------------------------------|
-| name                         | Arbitrary string to identify connection and identify it from other openid_connect providers                                                                                                                        | no       | String: openid_connect     | :my_idp                                             |
+| name                         | Arbitrary string to identify connection and identify it from other openid_connect providers                                                                   | no       | String: openid_connect     | :my_idp                                             |
 | issuer                       | Root url for the authorization server                                                                                                                         | yes      |                            | https://myprovider.com                              |
 | discovery                    | Should OpenID discovery be used. This is recommended if the IDP provides a discovery endpoint. See client config for how to manually enter discovered values. | no       | false                      | one of: true, false                                 |
 | client_auth_method           | Which authentication method to use to authenticate your app with the authorization server                                                                     | no       | Sym: basic                 | "basic", "jwks"                                     |
 | scope                        | Which OpenID scopes to include (:openid is always required)                                                                                                   | no       | Array<sym> [:openid]       | [:openid, :profile, :email]                         |
 | response_type                | Which OAuth2 response type to use with the authorization request                                                                                              | no       | String: code               | one of: 'code', 'id_token'                          |
+| require_state                | Should state param be verified - this is recommended, not required by the OIDC specification                                                                  | no       | true                       | false                                               |
 | state                        | A value to be used for the OAuth2 state parameter on the authorization request. Can be a proc that generates a string.                                        | no       | Random 16 character string | Proc.new { SecureRandom.hex(32) }                   |
 | response_mode                | The response mode per [spec](https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html)                                                              | no       | nil                        | one of: :query, :fragment, :form_post, :web_message |
 | display                      | An optional parameter to the authorization request to determine how the authorization and consent page                                                        | no       | nil                        | one of: :page, :popup, :touch, :wap                 |

lib/omniauth/strategies/openid_connect.rb

@@ -41,6 +41,7 @@ class OpenIDConnect
       option :client_x509_signing_key
       option :scope, [:openid]
       option :response_type, 'code' # ['code', 'id_token']
+      option :require_state, true
       option :state
       option :response_mode # [:query, :fragment, :form_post, :web_message]
       option :display, nil # [:page, :popup, :touch, :wap]
@@ -107,7 +108,7 @@ def request_phase
       def callback_phase
         error = params['error_reason'] || params['error']
         error_description = params['error_description'] || params['error_reason']
-        invalid_state = params['state'].to_s.empty? || params['state'] != stored_state
+        invalid_state = (options.require_state && params['state'].to_s.empty?) || params['state'] != stored_state
 
         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

test/lib/omniauth/strategies/openid_connect_test.rb

@@ -284,6 +284,66 @@ def test_callback_phase_with_discovery # rubocop:disable Metrics/AbcSize
         strategy.callback_phase
       end
 
+      def test_callback_phase_with_no_state_without_state_verification
+        code = SecureRandom.hex(16)
+        nonce = SecureRandom.hex(16)
+        jwks = JSON::JWK::Set.new(JSON.parse(File.read('test/fixtures/jwks.json'))['keys'])
+
+        strategy.options.require_state = false
+
+        request.stubs(:params).returns('code' => code)
+        request.stubs(:path).returns('')
+
+        strategy.options.client_options.host = 'example.com'
+        strategy.options.discovery = true
+
+        issuer = stub('OpenIDConnect::Discovery::Issuer')
+        issuer.stubs(:issuer).returns('https://example.com/')
+        ::OpenIDConnect::Discovery::Provider.stubs(:discover!).returns(issuer)
+
+        config = stub('OpenIDConnect::Discovery::Provder::Config')
+        config.stubs(:authorization_endpoint).returns('https://example.com/authorization')
+        config.stubs(:token_endpoint).returns('https://example.com/token')
+        config.stubs(:userinfo_endpoint).returns('https://example.com/userinfo')
+        config.stubs(:jwks_uri).returns('https://example.com/jwks')
+        config.stubs(:jwks).returns(jwks)
+
+        ::OpenIDConnect::Discovery::Provider::Config.stubs(:discover!).with('https://example.com/').returns(config)
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:raw_attributes).returns('sub' => 'sub', 'name' => 'name', 'email' => 'email')
+        id_token.stubs(:verify!).with(issuer: 'https://example.com/', client_id: @identifier, nonce: nonce).returns(true)
+        ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+
+        strategy.unstub(:user_info)
+        access_token = stub('OpenIDConnect::AccessToken')
+        access_token.stubs(:access_token)
+        access_token.stubs(:refresh_token)
+        access_token.stubs(:expires_in)
+        access_token.stubs(:scope)
+        access_token.stubs(:id_token).returns(File.read('test/fixtures/id_token.txt'))
+        client.expects(:access_token!).at_least_once.returns(access_token)
+        access_token.expects(:userinfo!).returns(user_info)
+
+        strategy.call!('rack.session' => { 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+      end
+
+      def test_callback_phase_with_invalid_state_without_state_verification
+        code = SecureRandom.hex(16)
+        state = SecureRandom.hex(16)
+        nonce = SecureRandom.hex(16)
+
+        strategy.options.require_state = false
+
+        request.stubs(:params).returns('code' => code, 'state' => 'foobar')
+        request.stubs(:path).returns('')
+
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.expects(:fail!)
+        strategy.callback_phase
+      end
+
       def test_callback_phase_with_error
         state = SecureRandom.hex(16)
         nonce = SecureRandom.hex(16)