Merge branch 'feat-http-timeout'

9 files changed, 55 insertions, 17 deletions

diff --git a/docs/howto/config.rst b/docs/howto/config.rst
index e2e4bc41..c224cb2a 100644
--- a/docs/howto/config.rst
+++ b/docs/howto/config.rst
@@ -113,6 +113,15 @@ Example::
 
 Whether debug information should be sent to the log file.
 
+http_client_timeout
+^^^^^^^^^^^^^^^^^^^
+
+Example::
+
+    http_client_timeout: 10
+
+The timeout of HTTP requests, in seconds. Defaults to None.
+
 additional_cert_files
 ^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/src/saml2/config.py b/src/saml2/config.py
index 357dc6b8..a0cc1ae4 100644
--- a/src/saml2/config.py
+++ b/src/saml2/config.py
@@ -76,6 +76,7 @@ COMMON_ARGS = [
     "name_id_format",
     "signing_algorithm",
     "digest_algorithm",
+    "http_client_timeout",
 ]
 
 SP_ARGS = [
@@ -228,6 +229,7 @@ class Config(object):
         self.delete_tmpfiles = True
         self.signing_algorithm = None
         self.digest_algorithm = None
+        self.http_client_timeout = None
 
     def setattr(self, context, attr, val):
         if context == "":
@@ -381,11 +383,13 @@ class Config(object):
             disable_validation = False
 
         mds = MetadataStore(
-            acs, self, ca_certs, disable_ssl_certificate_validation=disable_validation
+            acs,
+            self,
+            ca_certs,
+            disable_ssl_certificate_validation=disable_validation,
+            http_client_timeout=self.http_client_timeout,
         )
-
         mds.imp(metadata_conf)
-
         return mds
 
     def endpoint(self, service, binding=None, context=None):
diff --git a/src/saml2/ecp_client.py b/src/saml2/ecp_client.py
index 94cfe135..71533393 100644
--- a/src/saml2/ecp_client.py
+++ b/src/saml2/ecp_client.py
@@ -78,7 +78,8 @@ class Client(Entity):
         self._verbose = verbose
 
         if metadata_file:
-            self._metadata = MetadataStore([saml, samlp], None, config)
+            self._metadata = MetadataStore([saml, samlp], None, config,
+                                           http_client_timeout=config.http_client_timeout)
             self._metadata.load("local", metadata_file)
             logger.debug("Loaded metadata from '%s'", metadata_file)
         else:
diff --git a/src/saml2/entity.py b/src/saml2/entity.py
index 2d03c875..24cb95f6 100644
--- a/src/saml2/entity.py
+++ b/src/saml2/entity.py
@@ -165,7 +165,7 @@ class Entity(HTTPBase):
                 continue
 
             if _val.startswith("http"):
-                r = requests.request("GET", _val)
+                r = requests.request("GET", _val, timeout=self.config.http_client_timeout)
                 if r.status_code == 200:
                     tmp = make_temp(r.text, ".pem", False, self.config.delete_tmpfiles)
                     setattr(self.config, item, tmp.name)
@@ -175,7 +175,7 @@ class Entity(HTTPBase):
 
         HTTPBase.__init__(self, self.config.verify_ssl_cert,
                           self.config.ca_certs, self.config.key_file,
-                          self.config.cert_file)
+                          self.config.cert_file, self.config.http_client_timeout)
 
         if self.config.vorg:
             for vo in self.config.vorg.values():
diff --git a/src/saml2/httpbase.py b/src/saml2/httpbase.py
index f8393639..17c7373d 100644
--- a/src/saml2/httpbase.py
+++ b/src/saml2/httpbase.py
@@ -100,7 +100,7 @@ def dict2set_list(dic):
 
 class HTTPBase(object):
     def __init__(self, verify=True, ca_bundle=None, key_file=None,
-                 cert_file=None):
+                 cert_file=None, http_client_timeout=None):
         self.request_args = {"allow_redirects": False}
         #self.cookies = {}
         self.cookiejar = http_cookiejar.CookieJar()
@@ -111,6 +111,7 @@ class HTTPBase(object):
                 self.request_args["verify"] = ca_bundle
             if key_file:
                 self.request_args["cert"] = (cert_file, key_file)
+        self.request_args["timeout"] = http_client_timeout
 
         self.sec = None
         self.user = None
diff --git a/src/saml2/mdstore.py b/src/saml2/mdstore.py
index 120e4fc8..7aa458f8 100644
--- a/src/saml2/mdstore.py
+++ b/src/saml2/mdstore.py
@@ -928,7 +928,8 @@ class MetaDataMDX(InMemoryMetaData):
         return transform
 
     def __init__(self, url=None, security=None, cert=None,
-                 entity_transform=None, freshness_period=None, **kwargs):
+                 entity_transform=None, freshness_period=None,
+                 http_client_timeout=None, **kwargs):
         """
         :params url: mdx service url
         :params security: SecurityContext()
@@ -940,6 +941,7 @@ class MetaDataMDX(InMemoryMetaData):
         sha1 transformation.
         :params freshness_period: a duration in the format described at
         https://www.w3.org/TR/xmlschema-2/#duration
+        :params http_client_timeout: timeout of http requests
         """
         super(MetaDataMDX, self).__init__(None, **kwargs)
         if not url:
@@ -956,6 +958,7 @@ class MetaDataMDX(InMemoryMetaData):
         self.security = security
         self.freshness_period = freshness_period or DEFAULT_FRESHNESS_PERIOD
         self.expiration_date = {}
+        self.http_client_timeout = http_client_timeout
 
         # We assume that the MDQ server will return a single entity
         # described by a single <EntityDescriptor> element. The protocol
@@ -976,7 +979,8 @@ class MetaDataMDX(InMemoryMetaData):
             url=self.url, id=self.entity_transform(item)
         )
 
-        response = requests.get(mdx_url, headers={"Accept": SAML_METADATA_CONTENT_TYPE})
+        response = requests.get(mdx_url, headers={"Accept": SAML_METADATA_CONTENT_TYPE},
+                                timeout=self.http_client_timeout)
         if response.status_code != 200:
             error_msg = "Fething {item}: Got response status {status}".format(
                 item=item, status=response.status_code
@@ -1022,7 +1026,7 @@ class MetadataStore(MetaData):
     def __init__(self, attrc, config, ca_certs=None,
                  check_validity=True,
                  disable_ssl_certificate_validation=False,
-                 filter=None):
+                 filter=None, http_client_timeout=None):
         """
         :params attrc:
         :params config: Config()
@@ -1032,9 +1036,9 @@ class MetadataStore(MetaData):
         MetaData.__init__(self, attrc, check_validity=check_validity)
 
         if disable_ssl_certificate_validation:
-            self.http = HTTPBase(verify=False, ca_bundle=ca_certs)
+            self.http = HTTPBase(verify=False, ca_bundle=ca_certs, http_client_timeout=http_client_timeout)
         else:
-            self.http = HTTPBase(verify=True, ca_bundle=ca_certs)
+            self.http = HTTPBase(verify=True, ca_bundle=ca_certs, http_client_timeout=http_client_timeout)
 
         self.security = security_context(config)
         self.ii = 0
@@ -1042,6 +1046,7 @@ class MetadataStore(MetaData):
         self.check_validity = check_validity
         self.filter = filter
         self.to_old = {}
+        self.http_client_timeout = http_client_timeout
 
     def load(self, *args, **kwargs):
         if self.filter:
@@ -1100,11 +1105,12 @@ class MetadataStore(MetaData):
                 security = self.security
                 entity_transform = kwargs.get('entity_transform', None)
                 _md = MetaDataMDX(url, security, cert, entity_transform,
-                                  freshness_period=freshness_period)
+                                  freshness_period=freshness_period,
+                                  http_client_timeout=self.http_client_timeout)
             else:
                 key = args[1]
                 url = args[1]
-                _md = MetaDataMDX(url)
+                _md = MetaDataMDX(url, http_client_timeout=self.http_client_timeout)
         else:
             raise SAMLError("Unknown metadata type '%s'" % typ)
         _md.load()
diff --git a/tests/sp_1_conf.py b/tests/sp_1_conf.py
index 834ff15a..0b899ae4 100644
--- a/tests/sp_1_conf.py
+++ b/tests/sp_1_conf.py
@@ -48,4 +48,5 @@ CONFIG = {
                        },
     ],
     "secret": "0123456789",
+    "http_client_timeout": 10,
 }  
diff --git a/tests/test_30_mdstore.py b/tests/test_30_mdstore.py
index 8ce058b7..b35a206f 100644
--- a/tests/test_30_mdstore.py
+++ b/tests/test_30_mdstore.py
@@ -345,6 +345,16 @@ def test_mdx_service():
     assert len(certs) == 1
 
 
+@patch('saml2.httpbase.requests.get')
+def test_mdx_service_request_timeout(mock_request):
+    entity_id = "http://xenosmilus.umdc.umu.se/simplesaml/saml2/idp/metadata.php"
+    url = "http://mdx.example.com/entities/{}".format(MetaDataMDX.sha1_entity_transform(entity_id))
+
+    mdx = MetaDataMDX("http://mdx.example.com", http_client_timeout=10)
+    mdx.service(entity_id, "idpsso_descriptor", "single_sign_on_service")
+    mock_request.assert_called_with(url, headers={'Accept': 'application/samlmetadata+xml'}, timeout=10)
+
+
 @responses.activate
 def test_mdx_single_sign_on_service():
     entity_id = "http://xenosmilus.umdc.umu.se/simplesaml/saml2/idp/metadata.php"
@@ -462,12 +472,14 @@ def test_load_extern_incommon(mock_request):
 
     sec_config.xmlsec_binary = sigver.get_xmlsec_binary(["/opt/local/bin"])
     mds = MetadataStore(ATTRCONV, sec_config,
-                        disable_ssl_certificate_validation=True)
+                        disable_ssl_certificate_validation=True,
+                        http_client_timeout=10)
 
     mds.imp(METADATACONF["10"])
     print(mds)
     assert mds
     assert len(mds.keys())
+    mock_request.assert_called_with('GET', 'http://md.incommon.org/InCommon/InCommon-metadata-export.xml', allow_redirects=False, verify=False, timeout=10)
 
 
 def test_load_local():
@@ -495,10 +507,13 @@ def test_load_remote_encoding(mock_request):
 
     crypto = sigver._get_xmlsec_cryptobackend()
     sc = sigver.SecurityContext(crypto, key_type="", cert_type="")
-    httpc = HTTPBase()
-    mds = MetaDataExtern(ATTRCONV, 'http://metadata.aai.switch.ch/metadata.aaitest.xml', sc, full_path('SWITCHaaiRootCA.crt.pem'), httpc)
+    url = 'http://metadata.aai.switch.ch/metadata.aaitest.xml'
+    httpc = HTTPBase(http_client_timeout=10)
+    mds = MetaDataExtern(ATTRCONV, url, sc, full_path('SWITCHaaiRootCA.crt.pem'), httpc)
     mds.load()
 
+    mock_request.assert_called_with('GET', url, allow_redirects=False, verify=True, timeout=10)
+
 
 def test_load_string():
     sec_config.xmlsec_binary = sigver.get_xmlsec_binary(["/opt/local/bin"])
diff --git a/tests/test_31_config.py b/tests/test_31_config.py
index d58b9a01..e5847a31 100644
--- a/tests/test_31_config.py
+++ b/tests/test_31_config.py
@@ -338,6 +338,7 @@ def test_3():
     assert cnf.secret == "0123456789"
     assert cnf.metadata is not None
     assert cnf.attribute_converters is not None
+    assert cnf.http_client_timeout == 10
 
 
 def test_sp():