wip-NXDRIVE-2779-Investigate-the-login-issue-with-SSL-and-without-leg…

…acy-authentication-check-i.e.-oauth2 (#3540) * wip-NXDRIVE-2779-Investigate-the-login-issue-with-SSL-and-without-legacy-authentication-check-i.e.-oauth2 * wip-NXDRIVE-2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2. * NXDRIVE 2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2 -fix * NXDRIVE 2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2 -fix reqs update * NXDRIVE 2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2 -fix reqs update -fix mac-linux testcases * NXDRIVE 2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2 -fix reqs update -fix win testcases * NXDRIVE-2797: Update python-client dependency & other dependencies (#3650) * NXDRIVE-2797: Update python-client dependency * Bump types-python-dateutil from 0.1.4 to 2.8.19.2 in /tools/deps #3639 * Bump types-certifi from 0.1.4 to 2021.10.8.3 in /tools/deps #3501 Co-authored-by: mr-shekhar <[email protected]> * NXDRIVE 2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2 --final * NXDRIVE 2779: Investigate the login issue with SSL and without legacy authentication check i.e. oauth2 --final. Co-authored-by: Sweta Yadav <[email protected]>
nuxeo · Nov 9, 2022 · 092580d · 092580d
1 parent 695cdb1
commit 092580d
Show file tree

Hide file tree

Showing 11 changed files with 266 additions and 127 deletions.
diff --git a/docs/changes/5.3.0.md b/docs/changes/5.3.0.md
@@ -4,7 +4,7 @@ Release date: `2022-xx-xx`
 
 ## Core
 
-- [NXDRIVE-2](https://jira.nuxeo.com/browse/NXDRIVE-2):
+- [NXDRIVE-2779](https://jira.nuxeo.com/browse/NXDRIVE-2779):
 
 ### Direct Edit
 
@@ -32,7 +32,24 @@ Release date: `2022-xx-xx`
 
 ## Minor Changes
 
--
+- Added `charset-normalizer` 2.1.1
+- Upgraded `authlib` from 0.15.4 to 1.1.0
+- Upgraded `boto3` from 1.20.19 to 1.25.0
+- Upgraded `botocore` from 1.23.19 to 1.28.0
+- Upgraded `certifi` from 2021.5.30 to 2022.9.24
+- Upgraded `cffi` from 1.14.5 to 1.15.1
+- Upgraded `cryptography` from 3.4.7 to 38.0.2
+- Upgraded `idna` from 2.10 to 3.4
+- Upgraded `jmespath` from 0.10.0 to 1.0.1
+- Upgraded `jwt` from 1.2.0 to 1.3.1
+- Upgraded `nuxeo[oauth2,s3]` from 6.0.3 to 6.1.0
+- Upgraded `pycparser` from 2.20 to 2.21
+- Upgraded `python-dateutil` from 2.8.1 to 2.8.2
+- Upgraded `requests` from 2.25.1 to 2.28.1
+- Upgraded `s3transfer` from 0.5.0 to 0.6.0
+- Upgraded `urllib3` from 1.26.6 to 1.26.12
+- Upgraded `types-python-dateutil` from 0.1.4 to 2.8.19.2
+- Upgraded `types-certifi` from 0.1.4 to 2021.10.8.3
 
 ## Technical Changes
 

diff --git a/nxdrive/auth/oauth2.py b/nxdrive/auth/oauth2.py
@@ -4,6 +4,7 @@
 from nuxeo.client import Nuxeo
 
 from ..options import Options
+from ..utils import get_verify
 from .base import Authentication
 
 if TYPE_CHECKING:
@@ -44,16 +45,21 @@ def connect_url(self) -> str:
         return uri
 
     def get_token(self, **kwargs: Any) -> "Token":
+
         token: str = self.auth.request_token(
             code_verifier=kwargs["code_verifier"],
             code=kwargs["code"],
             state=kwargs["state"],
+            verify=get_verify(),
         )
+
         self.token = token
         return token
 
     def get_username(self) -> str:
-        client = Nuxeo(host=self.url, auth=self.auth)
-        user = client.users.current_user()
+
+        verification_needed = get_verify()
+        client = Nuxeo(host=self.url, auth=self.auth, verify=verification_needed)
+        user = client.users.current_user(verification_needed)
         username: str = user.uid
         return username
diff --git a/nxdrive/client/remote_client.py b/nxdrive/client/remote_client.py
@@ -64,6 +64,7 @@
 from ..utils import (
     compute_digest,
     get_current_locale,
+    get_verify,
     lock_path,
     shortify,
     sizeof_fmt,
@@ -115,13 +116,18 @@ def __init__(
             self.auth = BasicAuth(user_id, password)
             auth = self.auth
 
+        self.verification_needed = get_verify()
+        log.info(
+            f"SSL verify: {verify}-> will be changed to {self.verification_needed}"
+        )
+
         super().__init__(
             auth=auth,
             host=url,
             app_name=APP_NAME,
             version=version,
             repository=repository,
-            verify=verify,
+            verify=self.verification_needed,
             cert=cert,
         )
 
@@ -171,7 +177,7 @@ def __init__(
             self.base_folder_ref, self._base_folder_path = None, None
 
         # Cache the result for future uploads
-        self.uploads.has_s3()
+        self.uploads.has_s3(self.verification_needed)
 
         self.metrics = CustomPollMetrics(self)
         self.metrics.start()
@@ -279,7 +285,7 @@ def execute(self, /, **kwargs: Any) -> Any:
         """
         # Unauthorized and Forbidden exceptions are handled by the Python client.
         try:
-            return self.operations.execute(**kwargs)
+            return self.operations.execute(ssl_verify=Options.ssl_no_verify, **kwargs)
         except HTTPError as e:
             if e.status == requests.codes.not_found:
                 raise NotFound()
@@ -354,6 +360,7 @@ def request_token(self) -> Token:
                 TOKEN_PERMISSION,
                 app_name=APP_NAME,
                 device=current_os(full=True),
+                ssl_verify=self.verification_needed,
             )
             self.auth = self.client.auth
         else:
@@ -396,7 +403,10 @@ def download(
                 headers = {"Range": f"bytes={downloaded}-"}
 
         resp = self.client.request(
-            "GET", url.replace(self.client.host, ""), headers=headers
+            "GET",
+            url.replace(self.client.host, ""),
+            headers=headers,
+            ssl_verify=self.verification_needed,
         )
 
         if not file_out:
@@ -958,7 +968,9 @@ def set_proxy(self, proxy: Optional[Proxy], /) -> None:
     def get_server_configuration(self) -> Dict[str, Any]:
         try:
             return self.client.request(
-                "GET", f"{self.client.api_path}/drive/configuration"
+                "GET",
+                f"{self.client.api_path}/drive/configuration",
+                ssl_verify=get_verify(),
             ).json()
         except Exception as exc:
             log.warning(f"Error getting server configuration: {exc}")

diff --git a/nxdrive/engine/engine.py b/nxdrive/engine/engine.py
@@ -57,7 +57,7 @@
     force_decode,
     grouper,
     if_frozen,
-    requests_verify,
+    get_verify,
     safe_filename,
     safe_long_path,
     set_path_readonly,
@@ -1279,7 +1279,7 @@ def init_remote(self) -> Remote:
             "upload_callback": self.suspend_client,
             "dao": self.dao,
             "proxy": self.manager.proxy,
-            "verify": requests_verify(Options.ca_bundle, Options.ssl_no_verify),
+            "verify": get_verify(),
             "cert": client_certificate(),
         }
         return self.remote_cls(*args, **kwargs)

diff --git a/nxdrive/gui/application.py b/nxdrive/gui/application.py
@@ -74,11 +74,11 @@
     find_icon,
     find_resource,
     force_decode,
+    get_verify,
     if_frozen,
     normalize_event_filename,
     normalized_path,
     parse_protocol_url,
-    requests_verify,
     short_name,
     sizeof_fmt,
     today_is_special,
@@ -1098,11 +1098,13 @@ def auth() -> None:
             user = str(username.text())
             pwd = str(password.text())
 
+            verification_needed = get_verify()
+
             nuxeo = Nuxeo(
                 host=url,
                 auth=(user, pwd),
                 proxies=self.manager.proxy.settings(url=url),
-                verify=requests_verify(Options.ca_bundle, Options.ssl_no_verify),
+                verify=verification_needed,
                 cert=client_certificate(),
             )
             try:

diff --git a/nxdrive/utils.py b/nxdrive/utils.py
@@ -10,7 +10,7 @@
 import re
 import stat
 import sys
-from configparser import ConfigParser
+from configparser import DEFAULTSECT, ConfigParser
 from copy import deepcopy
 from datetime import datetime
 from functools import lru_cache
@@ -316,7 +316,7 @@ def get_value(value: str, /) -> Union[bool, float, str, Tuple[str, ...]]:
 
     if value.lower() in ("true", "1", "on", "yes", "oui"):
         return True
-    elif value.lower() in ("false", "0", "off", "no", "non"):
+    elif value.lower() in ("false", "0", "off", "no", "non", "none"):
         return False
     elif "\n" in value:
         return tuple(sorted(value.split()))
@@ -626,6 +626,9 @@ def client_certificate() -> Optional[Tuple[str, str]]:
     Fetch the paths to the certification file and it's key from the option.
     Return None if one of them is missing.
     """
+    if Options.ssl_no_verify is True:
+        return None
+
     client_certificate = (Options.cert_file, Options.cert_key_file)
     if not all(client_certificate):
         return None
@@ -752,6 +755,10 @@ def get_final_certificate_from_folder(folder: Path) -> Optional[Path]:
 
 def requests_verify(ca_bundle: Optional[Path], ssl_no_verify: bool) -> Any:
     """Return the appropriate value for the *verify* keyword argument of *requests* calls."""
+
+    if Options.ssl_no_verify and ssl_no_verify:
+        return False  # We do not want to verify ssl
+
     if ssl_no_verify:
         return False  # We do not want to verify ssl
 
@@ -1310,3 +1317,32 @@ def get_current_locale() -> str:
         l10n = locale.getdefaultlocale()[0] or ""
 
     return ".".join([l10n, encoding])
+
+
+def get_verify():
+    # type: () -> bool
+    """Detects if SSL verification is required or not"""
+    ssl_verification_needed = True
+    if Options.ssl_no_verify is True:
+        ssl_verification_needed = False
+    else:
+        ssl_verification_needed = requests_verify(
+            Options.ca_bundle, Options.ssl_no_verify
+        )
+
+    if ssl_verification_needed is not False:
+        try:
+            conf_file_path = get_config_path()
+            config = ConfigParser()
+            with conf_file_path.open(encoding="utf-8") as fh:
+                config.read_file(fh)
+            env = config.get(DEFAULTSECT, "env")
+            for key, value in config.items(env):
+                key = key.replace("-", "_").replace(".", "_").lower()
+                if key == "ssl_no_verify":
+                    ssl_verification_needed = False if get_value(value) else True
+        except Exception as exc:
+            log.info(f"Exception when trying to read config file: {exc!r}")
+            if "No such file or directory" and "-gw" in str(exc):
+                ssl_verification_needed = False
+    return ssl_verification_needed
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -5,6 +5,8 @@
 import nuxeo.operations
 import pytest
 from nuxeo.client import Nuxeo
+from nxdrive.options import Options
+from nxdrive.utils import get_verify
 
 from . import env
 
@@ -39,6 +41,10 @@ def pytest_runtest_makereport():
 def tmp(tmp_path):
     """Use the original *tmp_path* fixture with automatic clean-up."""
 
+    ssl_verify = get_verify()
+    if Options.ssl_no_verify != ssl_verify:
+        Options.ssl_no_verify = ssl_verify
+
     created_folders = []
     n = 0
 
@@ -84,6 +90,8 @@ def no_warnings(recwarn):
         elif "(rm_rf) error removing" in message:
             # First appeared with pytest 5.4.1
             continue
+        elif "Unverified HTTPS request is being made to host" in message:
+            continue
 
         warn = f"{warning.filename}:{warning.lineno} {message}"
         print(warn, file=sys.stderr)
@@ -141,8 +149,9 @@ def server(nuxeo_url):
     For now, we do not allow to use another than Administrator:Administrator
     to prevent unexpected actions on critical servers.
     """
+    verification_needed = get_verify()
     auth = (env.NXDRIVE_TEST_USERNAME, env.NXDRIVE_TEST_PASSWORD)
-    server = Nuxeo(host=nuxeo_url, auth=auth)
+    server = Nuxeo(auth=auth, host=nuxeo_url, verify=verification_needed)
     server.client.set(schemas=["dublincore"])
 
     # Save bandwidth by caching operations details
@@ -152,7 +161,7 @@ def server(nuxeo_url):
         nuxeo.operations.API.ops = OPS_CACHE
     global SERVER_INFO
     if not SERVER_INFO:
-        SERVER_INFO = server.client.server_info()
+        SERVER_INFO = server.client.server_info(ssl_verify=verification_needed)
         nuxeo.client.NuxeoClient._server_info = SERVER_INFO
 
     return server

diff --git a/tests/env.py b/tests/env.py
@@ -2,6 +2,7 @@
 
 # The server URL against to run tests
 NXDRIVE_TEST_NUXEO_URL = getenv("NXDRIVE_TEST_NUXEO_URL", "http://localhost:8080/nuxeo")
+# NXDRIVE_TEST_NUXEO_URL = getenv("NXDRIVE_TEST_NUXEO_URL", "https://localhost:8443/nuxeo")
 
 # The user having administrator rights
 NXDRIVE_TEST_USERNAME = getenv("NXDRIVE_TEST_USERNAME", "Administrator")

diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
@@ -412,7 +412,9 @@ def test_request_verify_ca_bundle_file(caplog, tmp_path):
 
     # Save the certificate for the first time
     caplog.clear()
-    final_certificate = Path(nxdrive.utils.requests_verify(ca_bundle, False))
+    cert = nxdrive.utils.requests_verify(ca_bundle, False)
+    path = "" if type(cert) == bool else cert
+    final_certificate = Path(path)
     records = [line.message for line in caplog.records]
     assert len(records) == 3
     assert "Saved the final certificate to" in records[0]
@@ -448,7 +450,9 @@ def test_request_verify_ca_bundle_file_is_str(caplog, tmp_path):
 
     # Save the certificate for the first time
     caplog.clear()
-    final_certificate = Path(nxdrive.utils.requests_verify(Options.ca_bundle, False))
+    cert = nxdrive.utils.requests_verify(ca_bundle, False)
+    path = "" if type(cert) == bool else cert
+    final_certificate = Path(path)
     records = [line.message for line in caplog.records]
     assert len(records) == 3
     assert "Saved the final certificate to" in records[0]
@@ -489,7 +493,9 @@ def test_request_verify_ca_bundle_file_mimic_updates(caplog, tmp_path):
 
     # Save the certificate for the first time
     caplog.clear()
-    final_certificate_1 = Path(nxdrive.utils.requests_verify(ca_bundle, False))
+    cert = nxdrive.utils.requests_verify(ca_bundle, False)
+    path = "" if type(cert) == bool else cert
+    final_certificate_1 = Path(path)
     records = [line.message for line in caplog.records]
     assert len(records) == 3
     assert "Saved the final certificate to" in records[0]

diff --git a/tools/deps/requirements-tests.txt b/tools/deps/requirements-tests.txt
@@ -332,17 +332,17 @@ typed-ast==1.4.3 \
     --hash=sha256:9c6d1a54552b5330bc657b7ef0eae25d00ba7ffe85d9ea8ae6540d2197a3788c \
     --hash=sha256:fb1bbeac803adea29cedd70781399c99138358c26d05fcbd23c13016b7f5ec65
     # via black
-types-certifi==0.1.4 \
-    --hash=sha256:afe4d94726491d843f10e5746797689ea5dcbd78454a653be47d72a8c8ce3bed \
-    --hash=sha256:7c134d978f15e4aa2d2b1a85b2a92241ed6b256c3452511b7783b6a28b304b71
+types-certifi==2021.10.8.3 \
+    --hash=sha256:72cf7798d165bc0b76e1c10dd1ea3097c7063c42c21d664523b928e88b554a4f \
+    --hash=sha256:b2d1e325e69f71f7c78e5943d410e650b4707bb0ef32e4ddf3da37f54176e88a
     # via mypy
 typing-extensions==4.0.1 \
     --hash=sha256:7f001e5ac290a0c0401508864c7ec868be4e701886d5b573a9528ed3973d9d3b \
     --hash=sha256:4ca091dea149f945ec56afb48dae714f21e8692ef22a395223bcd328961b6a0e
     # via mypy
-types-python-dateutil==0.1.4 \
-    --hash=sha256:39bfe0bde61fc673b8fa28167bd78622d976210f791971b9f3e10877cbf119a4 \
-    --hash=sha256:e6486ca27b6dde73e0ec079a9e1b03e208766e6bc7f1e08964a7e9104a5c7d7a
+types-python-dateutil==2.8.19.2 \
+    --hash=sha256:3f4dbe465e7e0c6581db11fd7a4855d1355b78712b3f292bd399cd332247e9c0 \
+    --hash=sha256:e6e32ce18f37765b08c46622287bc8d8136dc0c562d9ad5b8fd158c59963d7a7
     # via mypy
 types-PyYAML==5.4.3 \
     --hash=sha256:bca83cbfc0be48600a8abf1e3d87fb762a91e6d35d724029a3321dd2dce2ceb1 \