From 43407b636ba581ae225cb1414f34bcda16d60ae1 Mon Sep 17 00:00:00 2001 From: Manuel van Rijn Date: Fri, 24 May 2024 13:43:51 +0200 Subject: [PATCH 1/3] Add `audience` to the `client_options` I've come across an issue where the `identifier` wasn't equal to the `audience` in the token. This resulted in verification errors because currently it will verify the `aud` against the `identifier` if no `audience` is specified. In this PR, I introduced the `audience` as `client_options` and will pass this along in the `verify!` of the `decoded_id_token` so the openid_connect gem [can handle the expected audience](https://github.com/nov/openid_connect/blob/e1eb8ea962af43752b1aed2c1063a3e24f96c5bc/lib/openid_connect/response_object/id_token.rb#L30-L32) --- README.md | 1 + lib/omniauth/strategies/openid_connect.rb | 2 ++ .../strategies/openid_connect_test.rb | 20 +++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/README.md b/README.md index 169d908f..bae3f609 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,7 @@ These are the configuration options for the client_options hash of the configura | scheme | The http scheme to use | https | | | host | The host of the authorization server | nil | | | port | The port for the authorization server | 443 | | +| audience | The intended consumer of the token | nil | | | authorization_endpoint | The authorize endpoint on the authorization server | /authorize | yes | | token_endpoint | The token endpoint on the authorization server | /token | yes | | userinfo_endpoint | The user info endpoint on the authorization server | /userinfo | yes | diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index ebfaaa17..802d14bc 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -28,6 +28,7 @@ class OpenIDConnect # rubocop:disable Metrics/ClassLength scheme: 'https', host: nil, port: 443, + audience: nil, authorization_endpoint: '/authorize', token_endpoint: '/token', userinfo_endpoint: '/userinfo', @@ -466,6 +467,7 @@ def verify_id_token!(id_token) decode_id_token(id_token).verify!(issuer: options.issuer, client_id: client_options.identifier, + audience: client_options.audience, nonce: params['nonce'].presence || stored_nonce) end diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb index 031e1e3c..04eec1dc 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -248,6 +248,26 @@ def test_callback_phase_with_id_token strategy.callback_phase end + def test_callback_phase_with_audience + state = SecureRandom.hex(16) + strategy.options.response_type = 'id_token' + strategy.options.issuer = 'example.com' + strategy.options.client_options.audience = "my_audience" + + id_token = stub('OpenIDConnect::ResponseObject::IdToken') + id_token.expects(:verify!).with(issuer: strategy.options.issuer, client_id: @identifier, audience: "my_audience", nonce: nonce).returns(true) + id_token.stubs(:raw_attributes, :to_h).returns(payload) + + request.stubs(:params).returns('state' => state, 'nounce' => nonce, 'id_token' => id_token) + request.stubs(:path).returns('') + + strategy.stubs(:decode_id_token).returns(id_token) + strategy.stubs(:stored_state).returns(state) + + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + strategy.callback_phase + end + def test_callback_phase_with_id_token_and_param_provided_nonce # rubocop:disable Metrics/AbcSize code = SecureRandom.hex(16) state = SecureRandom.hex(16) From 94c649abd631b1069cf56a5985b2027e755f8174 Mon Sep 17 00:00:00 2001 From: Manuel van Rijn Date: Wed, 3 Jul 2024 14:14:05 +0200 Subject: [PATCH 2/3] Only pass along `audience` if it is specified --- lib/omniauth/strategies/openid_connect.rb | 12 ++++++++---- test/lib/omniauth/strategies/openid_connect_test.rb | 5 +++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index 802d14bc..0627392b 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -465,10 +465,14 @@ def configured_response_type def verify_id_token!(id_token) return unless id_token - decode_id_token(id_token).verify!(issuer: options.issuer, - client_id: client_options.identifier, - audience: client_options.audience, - nonce: params['nonce'].presence || stored_nonce) + verify_kwargs = { + issuer: options.issuer, + client_id: client_options.identifier, + nonce: params['nonce'].presence || stored_nonce, + } + verify_kwargs.merge!(audience: client_options.audience) if client_options.audience + + decode_id_token(id_token).verify!(**verify_kwargs) end class CallbackError < StandardError diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb index 04eec1dc..105edceb 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -252,10 +252,11 @@ def test_callback_phase_with_audience state = SecureRandom.hex(16) strategy.options.response_type = 'id_token' strategy.options.issuer = 'example.com' - strategy.options.client_options.audience = "my_audience" + strategy.options.client_options.audience = 'my_audience' id_token = stub('OpenIDConnect::ResponseObject::IdToken') - id_token.expects(:verify!).with(issuer: strategy.options.issuer, client_id: @identifier, audience: "my_audience", nonce: nonce).returns(true) + id_token.expects(:verify!).with(issuer: strategy.options.issuer, client_id: @identifier, audience: 'my_audience', + nonce: nonce).returns(true) id_token.stubs(:raw_attributes, :to_h).returns(payload) request.stubs(:params).returns('state' => state, 'nounce' => nonce, 'id_token' => id_token) From 38ae821b199d1dca9c9d22dd4af74aa23d0db0dd Mon Sep 17 00:00:00 2001 From: Manuel van Rijn Date: Fri, 5 Jul 2024 15:09:28 +0200 Subject: [PATCH 3/3] Update README.md Co-authored-by: Roger Meier --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bae3f609..93e5d928 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ These are the configuration options for the client_options hash of the configura | scheme | The http scheme to use | https | | | host | The host of the authorization server | nil | | | port | The port for the authorization server | 443 | | -| audience | The intended consumer of the token | nil | | +| audience | The intended consumer (`aud` field) of the id_token | nil | | | authorization_endpoint | The authorize endpoint on the authorization server | /authorize | yes | | token_endpoint | The token endpoint on the authorization server | /token | yes | | userinfo_endpoint | The user info endpoint on the authorization server | /userinfo | yes |