diff --git a/README.md b/README.md index e59b6c4f..668806eb 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Or install it yourself as: ## Supported Ruby Versions -OmniAuth::OpenIDConnect is tested under 2.7, 3.0, 3.1, 3.2, 3.3, 3.4 +OmniAuth::OpenIDConnect is tested under 2.7, 3.0, 3.1, 3.2, 3.3, 3.4, 4.0 ## Usage diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb index 73dd0fe0..0963b87d 100644 --- a/lib/omniauth/strategies/openid_connect.rb +++ b/lib/omniauth/strategies/openid_connect.rb @@ -110,11 +110,13 @@ def client end def config - @config ||= ::OpenIDConnect::Discovery::Provider::Config.discover!(options.issuer) + configure_discovery_scheme + @config ||= ::OpenIDConnect::Discovery::Provider::Config.discover!(discovery_base_url) end - def request_phase - options.issuer = issuer if options.issuer.to_s.empty? + def request_phase + options.issuer = issuer if issuer_empty? + discover! redirect authorize_uri end @@ -134,7 +136,7 @@ def callback_phase return unless valid_response_type? - options.issuer = issuer if options.issuer.nil? || options.issuer.empty? + options.issuer = issuer if issuer_empty? verify_id_token!(params['id_token']) if configured_response_type == 'id_token' discover! @@ -157,7 +159,7 @@ def callback_phase def other_phase if logout_path_pattern.match?(current_path) - options.issuer = issuer if options.issuer.to_s.empty? + options.issuer = issuer if issuer_empty? discover! return redirect(end_session_uri) if end_session_uri end @@ -250,6 +252,62 @@ def issuer ::OpenIDConnect::Discovery::Provider.discover!(resource).issuer end + # Helper method to safely check if issuer is empty + def issuer_empty? + options.issuer.nil? || options.issuer.to_s.empty? + end + + # When discovery is enabled we need a *stable* base URL. + # + # If the user provides an issuer without an explicit scheme (e.g. "keycloak:8080/realms/foo"), + # passing it to discovery can lead to unintended HTTPS/TLS attempts depending on underlying client defaults. + # + # Recommended behavior: only trust options.issuer for discovery if it is an absolute URI with http/https scheme. + # Otherwise, fall back to client_options.scheme/host/port. + def discovery_base_url + issuer = options.issuer.to_s + + if issuer.match?(/\Ahttps?:\/\//) + # Use the full issuer URL as-is for discovery + # The discovery endpoint will be: issuer + '/.well-known/openid-configuration' + issuer + else + # Issuer doesn't have a scheme, construct URL from client_options + resource = "#{client_options.scheme}://#{client_options.host}" + resource = "#{resource}:#{client_options.port}" if client_options.port + resource + end + end + + # Configure the SWD (Simple Web Discovery) library to use the correct URI scheme. + # + # The SWD library (used by openid_connect gem for discovery) defaults to URI::HTTPS, + # which causes SSL connection attempts even when an HTTP URL is explicitly provided. + # This results in "SSL_connect" errors when connecting to non-SSL servers (e.g., + # development Keycloak instances running on HTTP). + # + # This method fixes the issue by setting SWD.url_builder to URI::HTTP or URI::HTTPS + # based on the actual scheme of the discovery_base_url, ensuring the gem respects + # the explicitly configured protocol. + # + # See: https://github.com/nov/openid_connect/issues/47 + def configure_discovery_scheme + base_url = discovery_base_url + return if base_url.nil? || base_url.empty? + + uri = URI.parse(base_url) + + # Set SWD.url_builder to use HTTP or HTTPS based on the discovery URL scheme + # This prevents the gem from forcing HTTPS when HTTP is explicitly specified + if uri.scheme == 'http' + require 'swd' + SWD.url_builder = URI::HTTP + elsif uri.scheme == 'https' + require 'swd' + SWD.url_builder = URI::HTTPS + end + end + def discover! return unless options.discovery @@ -258,6 +316,9 @@ def discover! client_options.userinfo_endpoint = config.userinfo_endpoint client_options.jwks_uri = config.jwks_uri client_options.end_session_endpoint = config.end_session_endpoint if config.respond_to?(:end_session_endpoint) + + # Keep issuer consistent with what the provider advertises so later token verification uses the correct issuer. + options.issuer = config.issuer if config.respond_to?(:issuer) && config.issuer end def user_info diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb index dfc519ee..7dbb1881 100644 --- a/test/lib/omniauth/strategies/openid_connect_test.rb +++ b/test/lib/omniauth/strategies/openid_connect_test.rb @@ -127,6 +127,82 @@ def test_request_phase_with_discovery assert_nil strategy.options.client_options.end_session_endpoint end + def test_request_phase_with_discovery_http_scheme_and_port + expected_redirect = %r{^http://keycloak:8080/authorize\?client_id=1234&nonce=\w{32}&response_type=code&scope=openid&state=\w{32}$} + + strategy.options.client_options.scheme = 'http' + strategy.options.client_options.host = 'keycloak' + strategy.options.client_options.port = 8080 + strategy.options.discovery = true + + # Simulate provider discovery for issuer (used when options.issuer is empty) + issuer = stub('OpenIDConnect::Discovery::Issuer') + issuer.stubs(:issuer).returns('http://keycloak:8080/realms/quepid') + ::OpenIDConnect::Discovery::Provider.stubs(:discover!).with('http://keycloak:8080').returns(issuer) + + config = stub('OpenIDConnect::Discovery::Provider::Config') + config.stubs(:issuer).returns('http://keycloak:8080/realms/quepid') + config.stubs(:authorization_endpoint).returns('http://keycloak:8080/authorize') + config.stubs(:token_endpoint).returns('http://keycloak:8080/token') + config.stubs(:userinfo_endpoint).returns('http://keycloak:8080/userinfo') + config.stubs(:jwks_uri).returns('http://keycloak:8080/jwks') + ::OpenIDConnect::Discovery::Provider::Config.stubs(:discover!).with('http://keycloak:8080/realms/quepid').returns(config) + + strategy.expects(:redirect).with(regexp_matches(expected_redirect)) + strategy.request_phase + + assert_equal 'http://keycloak:8080/realms/quepid', strategy.options.issuer + assert_equal 'http://keycloak:8080/authorize', strategy.options.client_options.authorization_endpoint + end + + def test_discovery_ignores_scheme_less_issuer_and_falls_back_to_client_options + # issuer is present but scheme-less; discovery should not use it as base + strategy.options.issuer = 'keycloak:8080/realms/quepid' + strategy.options.discovery = true + strategy.options.client_options.scheme = 'http' + strategy.options.client_options.host = 'keycloak' + strategy.options.client_options.port = 8080 + + config = stub('OpenIDConnect::Discovery::Provider::Config') + config.stubs(:issuer).returns('http://keycloak:8080/realms/quepid') + config.stubs(:authorization_endpoint).returns('http://keycloak:8080/authorize') + config.stubs(:token_endpoint).returns('http://keycloak:8080/token') + config.stubs(:userinfo_endpoint).returns('http://keycloak:8080/userinfo') + config.stubs(:jwks_uri).returns('http://keycloak:8080/jwks') + + # key assertion: discover! uses http://keycloak:8080 (from client_options), NOT the scheme-less issuer + ::OpenIDConnect::Discovery::Provider::Config.expects(:discover!).with('http://keycloak:8080').returns(config) + + # call the private method via request_phase which triggers discover! + ::OpenIDConnect::Discovery::Provider.stubs(:discover!).returns(stub('OpenIDConnect::Discovery::Issuer', issuer: 'http://keycloak:8080/realms/quepid')) + + strategy.expects(:redirect) + strategy.request_phase + + assert_equal 'http://keycloak:8080/realms/quepid', strategy.options.issuer + end + + def test_configure_discovery_scheme_sets_swd_url_builder_for_http + require 'swd' + + # Test with HTTP scheme + strategy.options.issuer = 'http://keycloak:8080/realms/quepid' + strategy.options.discovery = true + + # Call the private method to configure discovery scheme + strategy.send(:configure_discovery_scheme) + + # Verify SWD.url_builder is set to URI::HTTP for HTTP URLs + assert_equal URI::HTTP, SWD.url_builder + + # Test with HTTPS scheme + strategy.options.issuer = 'https://example.com/realms/test' + strategy.send(:configure_discovery_scheme) + + # Verify SWD.url_builder is set to URI::HTTPS for HTTPS URLs + assert_equal URI::HTTPS, SWD.url_builder + end + def test_request_phase_with_response_mode expected_redirect = %r{^https://example\.com/authorize\?client_id=1234&nonce=\w{32}&response_mode=form_post&response_type=id_token&scope=openid&state=\w{32}$} strategy.options.issuer = 'example.com'