Skip to content

Commit

Permalink
Revert "fix: make require_state skip verification of state (#181)"
Browse files Browse the repository at this point in the history
This reverts commit 8d1f8ed.
  • Loading branch information
stanhu committed Jul 3, 2024
1 parent 8d1f8ed commit bd14191
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 33 deletions.
2 changes: 1 addition & 1 deletion lib/omniauth/strategies/openid_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def request_phase
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 = (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
Expand Down
34 changes: 2 additions & 32 deletions test/lib/omniauth/strategies/openid_connect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ def test_callback_phase_with_no_state_without_state_verification # rubocop:disab
strategy.callback_phase
end

def test_callback_phase_with_invalid_state_without_state_verification # rubocop:disable Metrics/AbcSize
def test_callback_phase_with_invalid_state_without_state_verification
code = SecureRandom.hex(16)
state = SecureRandom.hex(16)

Expand All @@ -505,38 +505,8 @@ def test_callback_phase_with_invalid_state_without_state_verification # rubocop:
request.stubs(:params).returns('code' => code, 'state' => 'foobar')
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(JSON::JWK::Set.new(jwks['keys']))

::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(jwt.to_s)
client.expects(:access_token!).at_least_once.returns(access_token)
access_token.expects(:userinfo!).returns(user_info)

strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
strategy.expects(:fail!)
strategy.callback_phase
end

Expand Down

0 comments on commit bd14191

Please sign in to comment.