Merge pull request #709 from johnnyshields/2.0-net-http-settings

Allow passing in timeout/retry settings to Net::HTTP
SAML-Toolkits · Jul 10, 2024 · f5768d2 · f5768d2
2 parents 8b4c340 + a53f3f9
commit f5768d2
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 * [#695](https://github.com/SAML-Toolkits/ruby-saml/pull/695) Deprecate `settings.compress_request` and `settings.compess_response` parameters.
 * [#690](https://github.com/SAML-Toolkits/ruby-saml/pull/690) Remove deprecated `settings.security[:embed_sign]` parameter.
 * [#697](https://github.com/SAML-Toolkits/ruby-saml/pull/697) Add deprecation for various parameters in `RubySaml::Settings`.
+* [#709](https://github.com/SAML-Toolkits/ruby-saml/pull/709) Allow passing in `Net::HTTP` `:open_timeout`, `:read_timeout`, and `:max_retries` settings to `IdpMetadataParser#parse_remote`.
 
 ### 1.17.0
 * [#687](https://github.com/SAML-Toolkits/ruby-saml/pull/687) Add CI coverage for Ruby 3.3 and Windows.

diff --git a/lib/ruby_saml/idp_metadata_parser.rb b/lib/ruby_saml/idp_metadata_parser.rb
@@ -61,12 +61,15 @@ def self.get_idps(metadata_document, only_entity_id=nil)
     # @option options [String, Array<String>, nil] :sso_binding an ordered list of bindings to detect the single signon URL. The first binding in the list that is included in the metadata will be used.
     # @option options [String, Array<String>, nil] :slo_binding an ordered list of bindings to detect the single logout URL. The first binding in the list that is included in the metadata will be used.
     # @option options [String, Array<String>, nil] :name_id_format an ordered list of NameIDFormats to detect a desired value. The first NameIDFormat in the list that is included in the metadata will be used.
+    # @option options [Numeric, nil] :open_timeout Number of seconds to wait for the connection to open. See Net::HTTP#open_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Numeric, nil] :read_timeout Number of seconds to wait for one block to be read. See Net::HTTP#read_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Integer, nil] :max_retries Maximum number of times to retry the request on certain errors. See Net::HTTP#max_retries= for more info. Default is the Net::HTTP default.
     #
     # @return [RubySaml::Settings]
     #
     # @raise [HttpError] Failure to fetch remote IdP metadata
     def parse_remote(url, validate_cert = true, options = {})
-      idp_metadata = get_idp_metadata(url, validate_cert)
+      idp_metadata = get_idp_metadata(url, validate_cert, options)
       parse(idp_metadata, options)
     end
 
@@ -80,6 +83,9 @@ def parse_remote(url, validate_cert = true, options = {})
     # @option options [String, Array<String>, nil] :sso_binding an ordered list of bindings to detect the single signon URL. The first binding in the list that is included in the metadata will be used.
     # @option options [String, Array<String>, nil] :slo_binding an ordered list of bindings to detect the single logout URL. The first binding in the list that is included in the metadata will be used.
     # @option options [String, Array<String>, nil] :name_id_format an ordered list of NameIDFormats to detect a desired value. The first NameIDFormat in the list that is included in the metadata will be used.
+    # @option options [Numeric, nil] :open_timeout Number of seconds to wait for the connection to open. See Net::HTTP#open_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Numeric, nil] :read_timeout Number of seconds to wait for one block to be read. See Net::HTTP#read_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Integer, nil] :max_retries Maximum number of times to retry the request on certain errors. See Net::HTTP#max_retries= for more info. Default is the Net::HTTP default.
     #
     # @return [Hash]
     #
@@ -98,12 +104,15 @@ def parse_remote_to_hash(url, validate_cert = true, options = {})
     # @option options [String, Array<String>, nil] :sso_binding an ordered list of bindings to detect the single signon URL. The first binding in the list that is included in the metadata will be used.
     # @option options [String, Array<String>, nil] :slo_binding an ordered list of bindings to detect the single logout URL. The first binding in the list that is included in the metadata will be used.
     # @option options [String, Array<String>, nil] :name_id_format an ordered list of NameIDFormats to detect a desired value. The first NameIDFormat in the list that is included in the metadata will be used.
+    # @option options [Numeric, nil] :open_timeout Number of seconds to wait for the connection to open. See Net::HTTP#open_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Numeric, nil] :read_timeout Number of seconds to wait for one block to be read. See Net::HTTP#read_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Integer, nil] :max_retries Maximum number of times to retry the request on certain errors. See Net::HTTP#max_retries= for more info. Default is the Net::HTTP default.
     #
     # @return [Array<Hash>]
     #
     # @raise [HttpError] Failure to fetch remote IdP metadata
     def parse_remote_to_array(url, validate_cert = true, options = {})
-      idp_metadata = get_idp_metadata(url, validate_cert)
+      idp_metadata = get_idp_metadata(url, validate_cert, options)
       parse_to_array(idp_metadata, options)
     end
 
@@ -188,9 +197,13 @@ def parse_to_idp_metadata_array(idp_metadata, options = {})
     # Retrieve the remote IdP metadata from the URL or a cached copy.
     # @param url [String] Url where the XML of the Identity Provider Metadata is published.
     # @param validate_cert [Boolean] If true and the URL is HTTPs, the cert of the domain is checked.
+    # @param options [Hash] Options used for requesting the remote URL
+    # @option options [Numeric, nil] :open_timeout Number of seconds to wait for the connection to open. See Net::HTTP#open_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Numeric, nil] :read_timeout Number of seconds to wait for one block to be read. See Net::HTTP#read_timeout for more info. Default is the Net::HTTP default.
+    # @option options [Integer, nil] :max_retries Maximum number of times to retry the request on certain errors. See Net::HTTP#max_retries= for more info. Default is the Net::HTTP default.
     # @return [REXML::document] Parsed XML IdP metadata
     # @raise [HttpError] Failure to fetch remote IdP metadata
-    def get_idp_metadata(url, validate_cert)
+    def get_idp_metadata(url, validate_cert, options = {})
       uri = URI.parse(url)
       raise ArgumentError.new("url must begin with http or https") unless /^https?/.match?(uri.scheme)
       http = Net::HTTP.new(uri.host, uri.port)
@@ -201,6 +214,10 @@ def get_idp_metadata(url, validate_cert)
         http.verify_mode = validate_cert ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE
       end
 
+      http.open_timeout = options[:open_timeout] if options[:open_timeout]
+      http.read_timeout = options[:read_timeout] if options[:read_timeout]
+      http.max_retries = options[:max_retries] if options[:max_retries]
+
       get = Net::HTTP::Get.new(uri.request_uri)
       get.basic_auth uri.user, uri.password if uri.user
       @response = http.request(get)

diff --git a/test/idp_metadata_parser_test.rb b/test/idp_metadata_parser_test.rb
@@ -329,12 +329,74 @@ def initialize; end
       assert_equal OpenSSL::SSL::VERIFY_PEER, @http.verify_mode
     end
 
-    it "accept self signed certificate if insturcted" do
+    it "accept self signed certificate if instructed" do
       idp_metadata_parser = RubySaml::IdpMetadataParser.new
       idp_metadata_parser.parse_remote(@url, false)
 
       assert_equal OpenSSL::SSL::VERIFY_NONE, @http.verify_mode
     end
+
+    it 'allows setting HTTP options on the request' do
+      refute_equal 2, @http.open_timeout
+      refute_equal 3, @http.read_timeout
+      refute_equal 4, @http.max_retries
+
+      idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new
+      idp_metadata_parser.parse_remote(@url, true, open_timeout: 2, read_timeout: 3, max_retries: 4)
+
+      assert_equal 2, @http.open_timeout
+      assert_equal 3, @http.read_timeout
+      assert_equal 4, @http.max_retries
+    end
+  end
+
+  describe "download and parse IdP descriptor file into an Array" do
+    before do
+      mock_response = MockSuccessResponse.new
+      mock_response.body = idp_metadata_descriptor
+      @url = "https://example.com"
+      uri = URI(@url)
+
+      @http = Net::HTTP.new(uri.host, uri.port)
+      Net::HTTP.expects(:new).returns(@http)
+      @http.expects(:request).returns(mock_response)
+    end
+
+    it "extract settings from remote xml" do
+      idp_metadata_parser = RubySaml::IdpMetadataParser.new
+      parsed_metadata = idp_metadata_parser.parse_remote_to_array(@url).first
+
+      assert_equal "https://hello.example.com/access/saml/idp.xml", parsed_metadata[:idp_entity_id]
+      assert_equal "https://hello.example.com/access/saml/login", parsed_metadata[:idp_sso_service_url]
+      assert_equal "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect", parsed_metadata[:idp_sso_service_binding]
+      assert_equal "C4:C6:BD:41:EC:AD:57:97:CE:7B:7D:80:06:C3:E4:30:53:29:02:0B:DD:2D:47:02:9E:BD:85:AD:93:02:45:21", parsed_metadata[:idp_cert_fingerprint]
+      assert_equal "https://hello.example.com/access/saml/logout", parsed_metadata[:idp_slo_service_url]
+      assert_equal "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect", parsed_metadata[:idp_slo_service_binding]
+      assert_equal "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified", parsed_metadata[:name_identifier_format]
+      assert_equal ["AuthToken", "SSOStartPage"], parsed_metadata[:idp_attribute_names]
+      assert_equal '2014-04-17T18:02:33.910Z', parsed_metadata[:valid_until]
+      assert_equal OpenSSL::SSL::VERIFY_PEER, @http.verify_mode
+    end
+
+    it "accept self signed certificate if instructed" do
+      idp_metadata_parser = RubySaml::IdpMetadataParser.new
+      idp_metadata_parser.parse_remote_to_array(@url, false)
+
+      assert_equal OpenSSL::SSL::VERIFY_NONE, @http.verify_mode
+    end
+
+    it 'allows setting HTTP options on the request' do
+      refute_equal 2, @http.open_timeout
+      refute_equal 3, @http.read_timeout
+      refute_equal 4, @http.max_retries
+
+      idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new
+      idp_metadata_parser.parse_remote_to_array(@url, true, open_timeout: 2, read_timeout: 3, max_retries: 4)
+
+      assert_equal 2, @http.open_timeout
+      assert_equal 3, @http.read_timeout
+      assert_equal 4, @http.max_retries
+    end
   end
 
   describe "download and parse IdP descriptor file into an Hash" do
@@ -365,12 +427,25 @@ def initialize; end
       assert_equal OpenSSL::SSL::VERIFY_PEER, @http.verify_mode
     end
 
-    it "accept self signed certificate if insturcted" do
+    it "accept self signed certificate if instructed" do
       idp_metadata_parser = RubySaml::IdpMetadataParser.new
       idp_metadata_parser.parse_remote_to_hash(@url, false)
 
       assert_equal OpenSSL::SSL::VERIFY_NONE, @http.verify_mode
     end
+
+    it 'allows setting HTTP options on the request' do
+      refute_equal 2, @http.open_timeout
+      refute_equal 3, @http.read_timeout
+      refute_equal 4, @http.max_retries
+
+      idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new
+      idp_metadata_parser.parse_remote_to_hash(@url, true, open_timeout: 2, read_timeout: 3, max_retries: 4)
+
+      assert_equal 2, @http.open_timeout
+      assert_equal 3, @http.read_timeout
+      assert_equal 4, @http.max_retries
+    end
   end
 
   describe "download failure cases" do