Merge pull request #299 from ricardobranco777/verify_tls

Verify TLS by default to avoid warnings

Makefile

@@ -16,7 +16,7 @@ pylint:
 
 .PHONY: flake8
 flake8:
-	flake8 --max-line-length=$(LINE_MAX) .
+	flake8 --max-line-length=$(LINE_MAX) $(FILES) tests/*.py
 
 .PHONY: test
 test:

ocw/lib/openqa.py

@@ -1,29 +1,36 @@
 from urllib.parse import urlparse
 from cachetools import cached
-from requests.exceptions import RequestException
+import requests
+from requests.exceptions import RequestException, SSLError
 import openqa_client.client
 from openqa_client.const import JOB_STATE_CANCELLED
 from openqa_client.exceptions import OpenQAClientError
 
 
-# We don't verify TLS server certificates because we
-# may encounter self-signed or expired certificates
-DEFAULT_VERIFY = False
+DEFAULT_TIMEOUT = 3
+
+
+def verify_tls(url: str) -> bool:
+    try:
+        requests.head(url, timeout=DEFAULT_TIMEOUT, verify=True).raise_for_status()
+    except (RequestException, SSLError):
+        return False
+    return True
 
 
 @cached(cache={})
-def get_url(server):
+def get_url(server: str) -> str:
     if server:
         server = server.rstrip('/').replace("_", ".")
     if urlparse(server).scheme != "":
         return server
     for scheme in ("https", "http"):
         try:
             url = f"{scheme}://{server}"
-            got = openqa_client.client.requests.head(
+            got = requests.head(
                 url,
-                timeout=5,
-                verify=DEFAULT_VERIFY,
+                timeout=DEFAULT_TIMEOUT,
+                verify=False,
             )
             got.raise_for_status()
             return url
@@ -45,7 +52,7 @@ def __new__(cls, **kwargs):
     def __init__(self, **kwargs):
         kwargs.pop("server")
         self.__client = openqa_client.client.OpenQA_Client(server=self.server, **kwargs)
-        self.__client.session.verify = DEFAULT_VERIFY
+        self.__client.session.verify = verify_tls(self.server)
 
     def is_cancelled(self, job_id: str) -> bool:
         if not job_id.isdigit():

tests/test_openqa.py

@@ -2,8 +2,8 @@
 import pytest
 from openqa_client.const import JOB_STATE_CANCELLED
 from openqa_client.exceptions import OpenQAClientError
-from requests.exceptions import RequestException
-from ocw.lib.openqa import OpenQA, get_url
+from requests.exceptions import RequestException, SSLError
+from ocw.lib.openqa import OpenQA, get_url, verify_tls
 
 
 @pytest.fixture
@@ -15,7 +15,8 @@ def openqa_client_mock():
 def openqa_instance(openqa_client_mock):
     with (
         patch('openqa_client.client.OpenQA_Client', return_value=openqa_client_mock),
-        patch('ocw.lib.openqa.get_url', return_value=None)
+        patch('ocw.lib.openqa.get_url', return_value=None),
+        patch('ocw.lib.openqa.verify_tls', return_value=None),
     ):
         yield OpenQA(server="myserver")
 
@@ -109,3 +110,26 @@ def test_get_url_with_invalid_server():
         with pytest.raises(OpenQAClientError):
             get_url("invalid-openqa.suse.de")
     get_url.cache_clear()
+
+
+# Create a mock response for requests.head
+class MockResponse:
+    def raise_for_status(self):
+        pass
+
+
+def test_verify_tls_valid_url():
+    with patch("requests.head", return_value=MockResponse()):
+        assert verify_tls("https://www.example.com")
+
+
+def test_verify_tls_invalid_url():
+    with patch("requests.head", side_effect=SSLError()):
+        result = verify_tls("https://www.invalidurl.com")
+        assert not result
+
+
+def test_verify_tls_request_exception():
+    with patch("requests.head", side_effect=RequestException()):
+        result = verify_tls("https://www.example.com")
+        assert not result