Fix: add requests timeout (#321)

.pre-commit-config.yaml

@@ -15,7 +15,7 @@ default_stages:
 
 repos:
   - repo: https://github.com/compilerla/conventional-pre-commit
-    rev: v2.1.1
+    rev: v2.4.0
     hooks:
       - id: conventional-pre-commit
         stages: [commit-msg]
@@ -33,27 +33,27 @@ repos:
       - id: check-added-large-files
 
   - repo: https://github.com/psf/black
-    rev: 23.1.0
+    rev: 23.9.1
     hooks:
       - id: black
         types:
           - python
 
   - repo: https://github.com/PyCQA/flake8
-    rev: 6.0.0
+    rev: 6.1.0
     hooks:
       - id: flake8
         types:
           - python
 
   - repo: https://github.com/pycqa/bandit
-    rev: 1.7.4
+    rev: 1.7.5
     hooks:
       - id: bandit
         args: ["-ll"]
         files: .py$
 
   - repo: https://github.com/igorshubovych/markdownlint-cli
-    rev: v0.33.0
+    rev: v0.36.0
     hooks:
       - id: markdownlint

eligibility_server/db/setup.py

@@ -64,7 +64,7 @@ def import_json_users(json_path, remote):
     data = {}
     if remote:
         # download the file to a dict
-        data = requests.get(json_path).json()
+        data = requests.get(json_path, timeout=config.request_timeout).json()
     else:
         # open the file and load to a dict
         with open(json_path) as file:
@@ -82,7 +82,7 @@ def import_csv_users(csv_path, remote):
     temp_csv = None
     if remote:
         # download the content as text and write to a temp file
-        content = requests.get(csv_path).text
+        content = requests.get(csv_path, timeout=config.request_timeout).text
         # note we leave the temp file open so it exists later for reading
         temp_csv = NamedTemporaryFile(mode="w", encoding="utf-8")
         temp_csv.write(content)

eligibility_server/keypair.py

@@ -17,7 +17,7 @@ def _read_key_file(key_path):
         return _CACHE[key_path]
 
     if key_path.startswith("http"):
-        data = requests.get(key_path).text
+        data = requests.get(key_path, timeout=config.request_timeout).text
         key = data.encode("utf8")
     else:
         with open(key_path, "rb") as pemfile:

eligibility_server/settings.py

@@ -9,6 +9,7 @@
 DEBUG_MODE = True
 HOST = "0.0.0.0"  # nosec
 LOG_LEVEL = "INFO"
+REQUEST_TIMEOUT = 5
 
 # Database settings
 
@@ -62,6 +63,10 @@ def host(self):
     def log_level(self):
         return str(current_app.config["LOG_LEVEL"])
 
+    @property
+    def request_timeout(self):
+        return int(current_app.config["REQUEST_TIMEOUT"])
+
     # API settings
 
     @property

tests/test_keypair.py

@@ -43,16 +43,16 @@ def test_read_key_file_local(mocker, sample_key_path_local, spy_open):
         assert key == file.read()
 
 
-@pytest.mark.usefixtures("reset_cache")
+@pytest.mark.usefixtures("reset_cache", "flask")
 def test_read_key_file_remote(sample_key_path_remote, spy_open, spy_requests_get):
     key = _read_key_file(sample_key_path_remote)
 
     # check that there was no call to open
     assert spy_open.call_count == 0
     # check that we made a get request
-    spy_requests_get.assert_called_with(sample_key_path_remote)
+    spy_requests_get.assert_called_once_with(sample_key_path_remote, timeout=5)
     assert key
-    assert key == requests.get(sample_key_path_remote).text.encode("utf8")
+    assert key == requests.get(sample_key_path_remote, timeout=5).text.encode("utf8")
 
 
 @pytest.mark.usefixtures("reset_cache")

tests/test_settings.py

@@ -51,6 +51,17 @@ def test_configuration_log_level(mocker, configuration: Configuration):
     assert configuration.log_level == new_value
 
 
+@pytest.mark.usefixtures("flask")
+def test_configuration_request_timeout(mocker, configuration: Configuration):
+    assert configuration.request_timeout == settings.REQUEST_TIMEOUT
+
+    new_value = 1000
+    mocked_config = {"REQUEST_TIMEOUT": new_value}
+    mocker.patch.dict("eligibility_server.settings.current_app.config", mocked_config)
+
+    assert configuration.request_timeout == new_value
+
+
 @pytest.mark.usefixtures("flask")
 def test_configuration_auth_header(mocker, configuration: Configuration):
     assert configuration.auth_header == settings.AUTH_HEADER