From 09af6dc7a39807daed52748568c23227218f6781 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Wed, 4 Jan 2023 09:42:49 -0500 Subject: [PATCH] Support `response_type: ["code", "id_token"]` Closes [omniauth/omniauth_openid_connect#105][] Similar to [omniauth/omniauth_openid_connect#107][] Some OpenID compatible IdP support hybrid authorizations that accept a `response_type` with both `code` and `id_token`. For example, [Microsoft Azure B2C][] accepts them as a URL-encoded array: > `response_type`: Must include an ID token for OpenID Connect. If your web application also needs tokens for calling a web API, you can use `code+id_token`. This commit extends the `OmniAuth::Strategies::OpenIDConnect` to encode the `response_type` into the query parameter as space-delimited token list when provided as an array. Similarly, when checking for missing keys in the response, iterate over the values as if they're an array. For the originally supported single-value case, the previous behavior is maintained. [Microsoft Azure B2C]: https://learn.microsoft.com/en-us/azure/active-directory-b2c/openid-connect#send-authentication-requests [omniauth/omniauth_openid_connect#105]: https://github.com/omniauth/omniauth_openid_connect/issues/105 [omniauth/omniauth_openid_connect#107]: https://github.com/omniauth/omniauth_openid_connect/pull/107 --- README.md | 2 +- lib/omniauth/strategies/openid_connect.rb | 14 +++--- .../strategies/openid_connect_test.rb | 46 +++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 4427d5e5..271eb141 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ config.omniauth :openid_connect, { | 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 [:openid] | [:openid, :profile, :email] | -| response_type | Which OAuth2 response type to use with the authorization request | no | String: code | one of: 'code', 'id_token' | +| response_type | Which OAuth2 response type to use with the authorization request | no | String or Array: code | 'code', 'id_token', or ['code', 'id_token'] | | 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) } | | require_state | Should state param be verified - this is recommended, not required by the OIDC specification | no | true | false | | 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 | diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index 52302b71..6bd1d8ba 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -125,11 +125,11 @@ def callback_phase options.issuer = issuer if options.issuer.nil? || options.issuer.empty? - verify_id_token!(params['id_token']) if configured_response_type == 'id_token' + verify_id_token!(params['id_token']) if configured_response_types.include?('id_token') discover! client.redirect_uri = redirect_uri - return id_token_callback_phase if configured_response_type == 'id_token' + return id_token_callback_phase if configured_response_types.include?('id_token') client.authorization_code = authorization_code access_token @@ -260,7 +260,7 @@ def access_token token_request_params[:code_verifier] = params['code_verifier'] || session.delete('omniauth.pkce.verifier') if options.pkce @access_token = client.access_token!(token_request_params) - verify_id_token!(@access_token.id_token) if configured_response_type == 'code' + verify_id_token!(@access_token.id_token) if configured_response_types.include?('code') @access_token end @@ -372,7 +372,9 @@ def id_token_callback_phase end def valid_response_type? - return true if params.key?(configured_response_type) + return true if configured_response_types.all? { |key| params.key?(key) } + + configured_response_type, * = configured_response_types error_attrs = RESPONSE_TYPE_EXCEPTIONS[configured_response_type] fail!(error_attrs[:key], error_attrs[:exception_class].new(params['error'])) @@ -380,8 +382,8 @@ def valid_response_type? false end - def configured_response_type - @configured_response_type ||= options.response_type.to_s + def configured_response_types + @configured_response_types ||= Array(options.response_type).map(&:to_s) end def verify_id_token!(id_token) diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb index 33c64c63..e62330d1 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -138,6 +138,28 @@ def test_request_phase_with_response_mode_symbol strategy.request_phase end + def test_request_phase_with_response_mode_array + expected_redirect = %r{^https://example\.com/authorize\?client_id=1234&nonce=\w{32}&response_mode=form_post&response_type=code%20id_token&scope=openid&state=\w{32}$} + strategy.options.issuer = 'example.com' + strategy.options.response_mode = 'form_post' + strategy.options.response_type = ['code', 'id_token'] + strategy.options.client_options.host = 'example.com' + + strategy.expects(:redirect).with(regexp_matches(expected_redirect)) + strategy.request_phase + end + + def test_request_phase_with_response_mode_symbol_array + expected_redirect = %r{^https://example\.com/authorize\?client_id=1234&nonce=\w{32}&response_mode=form_post&response_type=code%20id_token&scope=openid&state=\w{32}$} + strategy.options.issuer = 'example.com' + strategy.options.response_mode = 'form_post' + strategy.options.response_type = [:code, :id_token] + strategy.options.client_options.host = 'example.com' + + strategy.expects(:redirect).with(regexp_matches(expected_redirect)) + strategy.request_phase + end + def test_option_acr_values strategy.options.client_options[:host] = 'foobar.com' @@ -456,6 +478,30 @@ def test_callback_phase_without_id_token_symbol strategy.callback_phase end + def test_callback_phase_without_code_and_id_token + state = SecureRandom.hex(16) + request.stubs(:params).returns('state' => state) + request.stubs(:path).returns('') + strategy.options.response_type = ['code', 'id_token'] + + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + + strategy.expects(:fail!).with(:missing_code, is_a(OmniAuth::OpenIDConnect::MissingCodeError)) + strategy.callback_phase + end + + def test_callback_phase_without_code_and_id_token_symbol + state = SecureRandom.hex(16) + request.stubs(:params).returns('state' => state) + request.stubs(:path).returns('') + strategy.options.response_type = [:code, :id_token] + + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + + strategy.expects(:fail!).with(:missing_code, is_a(OmniAuth::OpenIDConnect::MissingCodeError)) + strategy.callback_phase + end + def test_callback_phase_with_timeout code = SecureRandom.hex(16) state = SecureRandom.hex(16)