diff --git a/README.md b/README.md index 4427d5e5..b98cd8f6 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ config.omniauth :openid_connect, { | pkce_verifier | Specify a custom PKCE verifier code. | no | A random 128-char string | Proc.new { SecureRandom.hex(64) } | | pkce_options | Specify a custom implementation of the PKCE code challenge/method. | no | SHA256(code_challenge) in hex | Proc to customise the code challenge generation | | client_options | A hash of client options detailed in its own section | yes | | | +| jwt_secret_base64 | For HMAC with SHA2 (e.g. HS256) signing algorithms, specify the base64-encoded secret used to sign the JWT token. Defaults to the OAuth2 client secret if not specified. | no | client_options.secret | "bXlzZWNyZXQ=\n" ### Client Config Options diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index 52302b71..60ad7c8b 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'base64' require 'timeout' require 'net/http' require 'open-uri' @@ -36,6 +37,7 @@ class OpenIDConnect # rubocop:disable Metrics/ClassLength option :issuer option :discovery, false option :client_signing_alg + option :jwt_secret_base64 option :client_jwk_signing_key option :client_x509_signing_key option :scope, [:openid] @@ -200,13 +202,19 @@ def authorize_uri def public_key @public_key ||= if options.discovery config.jwks - elsif key_or_secret - key_or_secret + elsif configured_public_key + configured_public_key elsif client_options.jwks_uri fetch_key end end + # Some OpenID providers use the OAuth2 client secret as the shared secret, but + # Keycloak uses a separate key that's stored inside the database. + def secret + base64_decoded_jwt_secret || client_options.secret + end + def pkce_authorize_params(verifier) # NOTE: see https://tools.ietf.org/html/rfc7636#appendix-A { @@ -221,6 +229,12 @@ def fetch_key @fetch_key ||= parse_jwk_key(::OpenIDConnect.http_client.get_content(client_options.jwks_uri)) end + def base64_decoded_jwt_secret + return unless options.jwt_secret_base64 + + Base64.decode64(options.jwt_secret_base64) + end + def issuer resource = "#{ client_options.scheme }://#{ client_options.host }" resource = "#{ resource }:#{ client_options.port }" if client_options.port @@ -265,8 +279,75 @@ def access_token @access_token end + # Unlike ::OpenIDConnect::ResponseObject::IdToken.decode, this + # method splits the decoding and verification of JWT into two + # steps. First, we decode the JWT without verifying it to + # determine the algorithm used to sign. Then, we verify it using + # the appropriate public key (e.g. if algorithm is RS256) or + # shared secret (e.g. if algorithm is HS256). This works around a + # limitation in the openid_connect gem: + # https://github.com/nov/openid_connect/issues/61 def decode_id_token(id_token) - ::OpenIDConnect::ResponseObject::IdToken.decode(id_token, public_key) + decoded = JSON::JWT.decode(id_token, :skip_verification) + algorithm = decoded.algorithm.to_sym + + validate_client_algorithm!(algorithm) + + keyset = + case algorithm + when :HS256, :HS384, :HS512 + secret + else + public_key + end + + decoded.verify!(keyset) + ::OpenIDConnect::ResponseObject::IdToken.new(decoded) + rescue JSON::JWK::Set::KidNotFound + # If the JWT has a key ID (kid), then we know that the set of + # keys supplied doesn't contain the one we want, and we're + # done. However, if there is no kid, then we try each key + # individually to see if one works: + # https://github.com/nov/json-jwt/pull/92#issuecomment-824654949 + raise if decoded&.header&.key?('kid') + + decoded = decode_with_each_key!(id_token, keyset) + + raise unless decoded + + decoded + end + + # If client_signing_alg is specified, we check that the returned JWT + # matches the expected algorithm. If not, we reject it. + def validate_client_algorithm!(algorithm) + client_signing_alg = options.client_signing_alg&.to_sym + + return unless client_signing_alg + return if algorithm == client_signing_alg + + reason = "Received JWT is signed with #{algorithm}, but client_singing_alg is configured for #{client_signing_alg}" + raise CallbackError, error: :invalid_jwt_algorithm, reason: reason, uri: params['error_uri'] + end + + def decode!(id_token, key) + ::OpenIDConnect::ResponseObject::IdToken.decode(id_token, key) + end + + def decode_with_each_key!(id_token, keyset) + return unless keyset.is_a?(JSON::JWK::Set) + + keyset.each do |key| + begin + decoded = decode!(id_token, key) + rescue JSON::JWS::VerificationFailed, JSON::JWS::UnexpectedAlgorithm, JSON::JWS::UnknownAlgorithm + next + end + + return decoded if decoded + end + + nil end def client_options @@ -308,18 +389,12 @@ def session super end - def key_or_secret - @key_or_secret ||= - case options.client_signing_alg&.to_sym - when :HS256, :HS384, :HS512 - client_options.secret - when :RS256, :RS384, :RS512 - if options.client_jwk_signing_key - parse_jwk_key(options.client_jwk_signing_key) - elsif options.client_x509_signing_key - parse_x509_key(options.client_x509_signing_key) - end - end + def configured_public_key + @configured_public_key ||= if options.client_jwk_signing_key + parse_jwk_key(options.client_jwk_signing_key) + elsif options.client_x509_signing_key + parse_x509_key(options.client_x509_signing_key) + end end def parse_x509_key(key) diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb index 33c64c63..3b53f78d 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -269,6 +269,138 @@ def test_callback_phase_with_id_token_and_param_provided_nonce # rubocop:disable strategy.callback_phase end + def test_callback_phase_with_id_token_no_kid + other_rsa_private = OpenSSL::PKey::RSA.generate(2048) + + key = JSON::JWK.new(private_key) + other_key = JSON::JWK.new(other_rsa_private) + state = SecureRandom.hex(16) + request.stubs(:params).returns('id_token' => jwt.to_s, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.client_signing_alg = :RS256 + strategy.options.client_jwk_signing_key = { 'keys' => [other_key, key] }.to_json + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + strategy.callback_phase + end + + def test_callback_phase_with_id_token_with_kid + other_rsa_private = OpenSSL::PKey::RSA.generate(2048) + + key = JSON::JWK.new(private_key) + other_key = JSON::JWK.new(other_rsa_private) + state = SecureRandom.hex(16) + jwt_with_kid = JSON::JWT.new(payload).sign(key, :RS256) + request.stubs(:params).returns('id_token' => jwt_with_kid.to_s, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.client_signing_alg = :RS256 + strategy.options.client_jwk_signing_key = { 'keys' => [other_key, key] }.to_json + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + strategy.callback_phase + end + + def test_callback_phase_with_id_token_with_kid_and_no_matching_kid + other_rsa_private = OpenSSL::PKey::RSA.generate(2048) + + key = JSON::JWK.new(private_key) + other_key = JSON::JWK.new(other_rsa_private) + state = SecureRandom.hex(16) + jwt_with_kid = JSON::JWT.new(payload).sign(key, :RS256) + request.stubs(:params).returns('id_token' => jwt_with_kid.to_s, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.client_signing_alg = :RS256 + # We use private_key here instead of the wrapped key, which contains a kid + strategy.options.client_jwk_signing_key = { 'keys' => [other_key, private_key] }.to_json + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + + assert_raises JSON::JWK::Set::KidNotFound do + strategy.callback_phase + end + end + + def test_callback_phase_with_id_token_with_hs256 + state = SecureRandom.hex(16) + request.stubs(:params).returns('id_token' => jwt_with_hs256.to_s, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.client_options.secret = hmac_secret + strategy.options.client_signing_alg = :HS256 + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + strategy.callback_phase + end + + def test_callback_phase_with_hs256_base64_jwt_secret + state = SecureRandom.hex(16) + request.stubs(:params).returns('id_token' => jwt_with_hs256.to_s, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.jwt_secret_base64 = Base64.encode64(hmac_secret) + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + strategy.callback_phase + end + + def test_callback_phase_with_mismatched_signing_algorithm + state = SecureRandom.hex(16) + request.stubs(:params).returns('id_token' => jwt_with_hs512.to_s, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.client_options.secret = hmac_secret + strategy.options.client_signing_alg = :HS256 + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + + strategy.expects(:fail!).with(:invalid_jwt_algorithm, is_a(OmniAuth::Strategies::OpenIDConnect::CallbackError)) + strategy.callback_phase + end + + def test_callback_phase_with_id_token_no_matching_key + rsa_private = OpenSSL::PKey::RSA.generate(2048) + other_rsa_private = OpenSSL::PKey::RSA.generate(2048) + + other_key = JSON::JWK.new(other_rsa_private) + token = JSON::JWT.new(payload).sign(rsa_private, :RS256).to_s + state = SecureRandom.hex(16) + request.stubs(:params).returns('id_token' => token, 'state' => state) + request.stubs(:path_info).returns('') + + strategy.options.issuer = issuer + strategy.options.client_signing_alg = :RS256 + strategy.options.client_jwk_signing_key = { 'keys' => [other_key] }.to_json + strategy.options.response_type = 'id_token' + + strategy.unstub(:user_info) + strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce }) + + assert_raises JSON::JWK::Set::KidNotFound do + strategy.callback_phase + end + end + def test_callback_phase_with_discovery # rubocop:disable Metrics/AbcSize state = SecureRandom.hex(16) @@ -699,7 +831,7 @@ def test_public_key_with_x509 def test_public_key_with_hmac strategy.options.client_options.secret = 'secret' strategy.options.client_signing_alg = :HS256 - assert_equal strategy.options.client_options.secret, strategy.public_key + assert_equal strategy.options.client_options.secret, strategy.secret end def test_id_token_auth_hash diff --git a/test/strategy_test_case.rb b/test/strategy_test_case.rb index f12ca8e4..1492d02b 100644 --- a/test/strategy_test_case.rb +++ b/test/strategy_test_case.rb @@ -37,6 +37,18 @@ def jwt @jwt ||= JSON::JWT.new(payload).sign(private_key, :RS256) end + def hmac_secret + @hmac_secret ||= SecureRandom.hex(16) + end + + def jwt_with_hs256 + @jwt_with_hs256 ||= JSON::JWT.new(payload).sign(hmac_secret, :HS256) + end + + def jwt_with_hs512 + @jwt_with_hs512 ||= JSON::JWT.new(payload).sign(hmac_secret, :HS512) + end + def jwks @jwks ||= begin key = JSON::JWK.new(private_key)