Merge branch 'master' into create_invitation_method

Author: harminius

mergin/client.py

@@ -19,7 +19,14 @@
 
 from typing import List
 
-from .common import ClientError, LoginError, WorkspaceRole, ProjectRole, LOG_FILE_SIZE_TO_SEND, MERGIN_DEFAULT_LOGS_URL
+from .common import (
+    ClientError,
+    LoginError,
+    WorkspaceRole,
+    ProjectRole,
+    MAX_LOG_FILE_SIZE_TO_SEND,
+    MERGIN_DEFAULT_LOGS_URL,
+)
 from .merginproject import MerginProject
 from .client_pull import (
     download_file_finalize,
@@ -94,7 +101,7 @@ def __init__(
         proxy_config=None,
     ):
         self.url = url if url is not None else MerginClient.default_url()
-        self._auth_params = None
+        self._auth_params = {}
         self._auth_session = None
         self._user_info = None
         self._server_type = None
@@ -192,36 +199,32 @@ def user_agent_info(self):
             system_version = platform.mac_ver()[0]
         return f"{self.client_version} ({platform.system()}/{system_version})"
 
-    def _check_token(f):
-        """Wrapper for creating/renewing authorization token."""
-
-        def wrapper(self, *args):
-            if self._auth_params:
-                if self._auth_session:
-                    # Refresh auth token if it expired or will expire very soon
-                    delta = self._auth_session["expire"] - datetime.now(timezone.utc)
-                    if delta.total_seconds() < 5:
-                        self.log.info("Token has expired - refreshing...")
-                        if self._auth_params.get("login", None) and self._auth_params.get("password", None):
-                            self.log.info("Token has expired - refreshing...")
-                            self.login(self._auth_params["login"], self._auth_params["password"])
-                        else:
-                            raise AuthTokenExpiredError("Token has expired - please re-login")
-                else:
-                    # Create a new authorization token
-                    self.log.info(f"No token - login user: {self._auth_params['login']}")
-                    if self._auth_params.get("login", None) and self._auth_params.get("password", None):
-                        self.login(self._auth_params["login"], self._auth_params["password"])
-                    else:
-                        raise ClientError("Missing login or password")
-
-            return f(self, *args)
+    def validate_auth(self):
+        """Validate that client has valid auth token or can be logged in."""
 
-        return wrapper
+        if self._auth_session:
+            # Refresh auth token if it expired or will expire very soon
+            delta = self._auth_session["expire"] - datetime.now(timezone.utc)
+            if delta.total_seconds() < 5:
+                self.log.info("Token has expired - refreshing...")
+                if self._auth_params.get("login", None) and self._auth_params.get("password", None):
+                    self.log.info("Token has expired - refreshing...")
+                    self.login(self._auth_params["login"], self._auth_params["password"])
+                else:
+                    raise AuthTokenExpiredError("Token has expired - please re-login")
+        else:
+            # Create a new authorization token
+            self.log.info(f"No token - login user: {self._auth_params.get('login', None)}")
+            if self._auth_params.get("login", None) and self._auth_params.get("password", None):
+                self.login(self._auth_params["login"], self._auth_params["password"])
+            else:
+                raise ClientError("Missing login or password")
 
-    @_check_token
-    def _do_request(self, request):
+    def _do_request(self, request, validate_auth=True):
         """General server request method."""
+        if validate_auth:
+            self.validate_auth()
+
         if self._auth_session:
             request.add_header("Authorization", self._auth_session["token"])
         request.add_header("User-Agent", self.user_agent_info())
@@ -263,31 +266,31 @@ def _do_request(self, request):
             # e.g. when DNS resolution fails (no internet connection?)
             raise ClientError("Error requesting " + request.full_url + ": " + str(e))
 
-    def get(self, path, data=None, headers={}):
+    def get(self, path, data=None, headers={}, validate_auth=True):
         url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
         if data:
             url += "?" + urllib.parse.urlencode(data)
         request = urllib.request.Request(url, headers=headers)
-        return self._do_request(request)
+        return self._do_request(request, validate_auth=validate_auth)
 
-    def post(self, path, data=None, headers={}):
+    def post(self, path, data=None, headers={}, validate_auth=True):
         url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
         if headers.get("Content-Type", None) == "application/json":
             data = json.dumps(data, cls=DateTimeEncoder).encode("utf-8")
         request = urllib.request.Request(url, data, headers, method="POST")
-        return self._do_request(request)
+        return self._do_request(request, validate_auth=validate_auth)
 
-    def patch(self, path, data=None, headers={}):
+    def patch(self, path, data=None, headers={}, validate_auth=True):
         url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
         if headers.get("Content-Type", None) == "application/json":
             data = json.dumps(data, cls=DateTimeEncoder).encode("utf-8")
         request = urllib.request.Request(url, data, headers, method="PATCH")
-        return self._do_request(request)
+        return self._do_request(request, validate_auth=validate_auth)
 
-    def delete(self, path):
+    def delete(self, path, validate_auth=True):
         url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
         request = urllib.request.Request(url, method="DELETE")
-        return self._do_request(request)
+        return self._do_request(request, validate_auth=validate_auth)
 
     def login(self, login, password):
         """
@@ -303,26 +306,16 @@ def login(self, login, password):
         self._auth_session = None
         self.log.info(f"Going to log in user {login}")
         try:
-            self._auth_params = params
-            url = urllib.parse.urljoin(self.url, urllib.parse.quote("/v1/auth/login"))
-            data = json.dumps(self._auth_params, cls=DateTimeEncoder).encode("utf-8")
-            request = urllib.request.Request(url, data, {"Content-Type": "application/json"}, method="POST")
-            request.add_header("User-Agent", self.user_agent_info())
-            resp = self.opener.open(request)
+            resp = self.post(
+                "/v1/auth/login", data=params, headers={"Content-Type": "application/json"}, validate_auth=False
+            )
             data = json.load(resp)
             session = data["session"]
-        except urllib.error.HTTPError as e:
-            if e.headers.get("Content-Type", "") == "application/problem+json":
-                info = json.load(e)
-                self.log.info(f"Login problem: {info.get('detail')}")
-                raise LoginError(info.get("detail"))
-            self.log.info(f"Login problem: {e.read().decode('utf-8')}")
-            raise LoginError(e.read().decode("utf-8"))
-        except urllib.error.URLError as e:
-            # e.g. when DNS resolution fails (no internet connection?)
-            raise ClientError("failure reason: " + str(e.reason))
+        except ClientError as e:
+            self.log.info(f"Login problem: {e.detail}")
+            raise LoginError(e.detail)
         self._auth_session = {
-            "token": "Bearer %s" % session["token"],
+            "token": f"Bearer {session['token']}",
             "expire": dateutil.parser.parse(session["expire"]),
         }
         self._user_info = {"username": data["username"]}
@@ -367,7 +360,7 @@ def server_type(self):
         """
         if not self._server_type:
             try:
-                resp = self.get("/config")
+                resp = self.get("/config", validate_auth=False)
                 config = json.load(resp)
                 if config["server_type"] == "ce":
                     self._server_type = ServerType.CE
@@ -389,7 +382,7 @@ def server_version(self):
         """
         if self._server_version is None:
             try:
-                resp = self.get("/config")
+                resp = self.get("/config", validate_auth=False)
                 config = json.load(resp)
                 self._server_version = config["version"]
             except (ClientError, KeyError):
@@ -1403,7 +1396,7 @@ def remove_project_collaborator(self, project_id: str, user_id: int):
 
     def server_config(self) -> dict:
         """Get server configuration as dictionary."""
-        response = self.get("/config")
+        response = self.get("/config", validate_auth=False)
         return json.load(response)
 
     def send_logs(
@@ -1441,16 +1434,20 @@ def send_logs(
                 platform.system(), self.url, self.username()
             )
 
+        # We send more from the local logs
+        global_logs_file_size_to_send = MAX_LOG_FILE_SIZE_TO_SEND * 0.2
+        local_logs_file_size_to_send = MAX_LOG_FILE_SIZE_TO_SEND * 0.8
+
         global_logs = b""
         if global_log_file and os.path.exists(global_log_file):
             with open(global_log_file, "rb") as f:
-                if os.path.getsize(global_log_file) > LOG_FILE_SIZE_TO_SEND:
-                    f.seek(-LOG_FILE_SIZE_TO_SEND, os.SEEK_END)
+                if os.path.getsize(global_log_file) > global_logs_file_size_to_send:
+                    f.seek(-global_logs_file_size_to_send, os.SEEK_END)
                 global_logs = f.read() + b"\n--------------------------------\n\n"
 
         with open(logfile, "rb") as f:
-            if os.path.getsize(logfile) > 512 * 1024:
-                f.seek(-512 * 1024, os.SEEK_END)
+            if os.path.getsize(logfile) > local_logs_file_size_to_send:
+                f.seek(-local_logs_file_size_to_send, os.SEEK_END)
             logs = f.read()
 
         payload = meta.encode() + global_logs + logs

mergin/common.py

@@ -7,7 +7,7 @@
 UPLOAD_CHUNK_SIZE = 10 * 1024 * 1024
 
 # size of the log file part to send (if file is larger only this size from end will be sent)
-LOG_FILE_SIZE_TO_SEND = 100 * 1024
+MAX_LOG_FILE_SIZE_TO_SEND = 8 * 1024 * 1024
 
 # default URL for submitting logs
 MERGIN_DEFAULT_LOGS_URL = "https://g4pfq226j0.execute-api.eu-west-1.amazonaws.com/mergin_client_log_submit"

mergin/test/test_client.py

@@ -5,7 +5,7 @@
 import tempfile
 import subprocess
 import shutil
-from datetime import datetime, timedelta, date
+from datetime import datetime, timedelta, date, timezone
 import pytest
 import pytz
 import sqlite3
@@ -14,6 +14,7 @@
 from .. import InvalidProject
 from ..client import (
     MerginClient,
+    AuthTokenExpiredError,
     ClientError,
     MerginProject,
     LoginError,
@@ -2887,8 +2888,7 @@ def test_mc_without_login():
     with pytest.raises(ClientError) as e:
         mc.workspaces_list()
 
-    assert e.value.http_error == 401
-    assert e.value.detail == '"Authentication information is missing or invalid."\n'
+    assert e.value.detail == "Missing login or password"
 
 
 def test_do_request_error_handling(mc: MerginClient):
@@ -2926,3 +2926,61 @@ def test_create_invitation(mc: MerginClient):
     assert inv["email"] == email
     assert "projects" not in inv
     mc.delete(f"v1/workspace/invitation/{inv['id']}")
+
+    
+def test_validate_auth(mc: MerginClient):
+    """Test validate authentication under different scenarios."""
+
+    # ----- Client without authentication -----
+    mc_not_auth = MerginClient(SERVER_URL)
+
+    with pytest.raises(ClientError) as e:
+        mc_not_auth.validate_auth()
+
+    assert e.value.detail == "Missing login or password"
+
+    # ----- Client with token -----
+    # create a client with valid auth token based on other MerginClient instance, but not with username/password
+    mc_auth_token = MerginClient(SERVER_URL, auth_token=mc._auth_session["token"])
+
+    # this should pass and not raise an error
+    mc_auth_token.validate_auth()
+
+    # manually set expire date to the past to simulate expired token
+    mc_auth_token._auth_session["expire"] = datetime.now(timezone.utc) - timedelta(days=1)
+
+    # check that this raises an error
+    with pytest.raises(AuthTokenExpiredError):
+        mc_auth_token.validate_auth()
+
+    # ----- Client with token and username/password -----
+    # create a client with valid auth token based on other MerginClient instance with username/password that allows relogin if the token is expired
+    mc_auth_token_login = MerginClient(
+        SERVER_URL, auth_token=mc._auth_session["token"], login=API_USER, password=USER_PWD
+    )
+
+    # this should pass and not raise an error
+    mc_auth_token_login.validate_auth()
+
+    # manually set expire date to the past to simulate expired token
+    mc_auth_token_login._auth_session["expire"] = datetime.now(timezone.utc) - timedelta(days=1)
+
+    # this should pass and not raise an error, as the client is able to re-login
+    mc_auth_token_login.validate_auth()
+
+    # ----- Client with token and username/WRONG password -----
+    # create a client with valid auth token based on other MerginClient instance with username and WRONG password
+    # that does NOT allow relogin if the token is expired
+    mc_auth_token_login_wrong_password = MerginClient(
+        SERVER_URL, auth_token=mc._auth_session["token"], login=API_USER, password="WRONG_PASSWORD"
+    )
+
+    # this should pass and not raise an error
+    mc_auth_token_login_wrong_password.validate_auth()
+
+    # manually set expire date to the past to simulate expired token
+    mc_auth_token_login_wrong_password._auth_session["expire"] = datetime.now(timezone.utc) - timedelta(days=1)
+
+    # this should pass and not raise an error, as the client is able to re-login
+    with pytest.raises(LoginError):
+        mc_auth_token_login_wrong_password.validate_auth()

mergin/version.py

@@ -1,5 +1,5 @@
 # The version is also stored in ../setup.py
-__version__ = "0.10.1"
+__version__ = "0.10.2"
 
 # There seems to be no single nice way to keep version info just in one place:
 # https://packaging.python.org/guides/single-sourcing-package-version/

setup.py

@@ -5,7 +5,7 @@
 
 setup(
     name='mergin-client',
-    version='0.10.1',
+    version='0.10.2',
     url='https://github.com/MerginMaps/python-api-client',
     license='MIT',
     author='Lutra Consulting Ltd.',