Merge branch 'issue198-permission-issues-with-privatejsonfile-on-windows'

Author: soxofaan

CHANGELOG.md

@@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed/improved math operator/process support for `DataCube`s in "apply" mode (non-"band math"),
   allowing expressions like `10 * cube.log10()` and `~(cube == 0)`
   ([#123](https://github.com/Open-EO/openeo-python-client/issues/123))
+- Support `PrivateJsonFile` permissions properly on Windows, using oschmod library.
+  ([#198](https://github.com/Open-EO/openeo-python-client/issues/198))
+- Fixed some broken unit tests on Windows related to path (separator) handling.
+  ([#350](https://github.com/Open-EO/openeo-python-client/issues/350))
 
 
 ## [0.13.0] - 2022-10-10 - "UDF UX" release

openeo/rest/auth/config.py

@@ -8,7 +8,6 @@
 
 import json
 import logging
-import platform
 import stat
 from datetime import datetime
 from pathlib import Path
@@ -18,22 +17,42 @@
 from openeo.config import get_user_config_dir, get_user_data_dir
 from openeo.util import rfc3339, deep_get, deep_set
 
+try:
+    # Use oschmod when available (fall back to POSIX-only functionality from stdlib otherwise)
+    # TODO: enforce oschmod as dependency for all platforms?
+    import oschmod
+except ImportError:
+    oschmod = None
+
+
 _PRIVATE_PERMS = stat.S_IRUSR | stat.S_IWUSR
 
 log = logging.getLogger(__name__)
 
 
+def get_file_mode(path: Path) -> int:
+    """Get the file permission bits in a way that works on both *nix and Windows platforms."""
+    if oschmod:
+        return oschmod.get_mode(str(path))
+    return path.stat().st_mode
+
+
+def set_file_mode(path: Path, mode: int):
+    """Set the file permission bits in a way that works on both *nix and Windows platforms."""
+    if oschmod:
+        oschmod.set_mode(str(path), mode=mode)
+    else:
+        path.chmod(mode=mode)
+
+
 def assert_private_file(path: Path):
     """Check that given file is only readable by user."""
-    mode = path.stat().st_mode
+    mode = get_file_mode(path)
     if (mode & stat.S_IRWXG) or (mode & stat.S_IRWXO):
         message = "File {p} could be readable by others: mode {a:o} (expected: {e:o}).".format(
             p=path, a=mode & (stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO), e=_PRIVATE_PERMS
         )
-        if platform.system() == 'Windows':
-            log.info(message)
-        else:
-            raise PermissionError(message)
+        raise PermissionError(message)
 
 
 def utcnow_rfc3339() -> str:
@@ -86,7 +105,7 @@ def _write(self, data: dict):
         # TODO: add file locking to avoid race conditions?
         with self._path.open("w", encoding="utf8") as f:
             json.dump(data, f, indent=2)
-        self._path.chmod(mode=_PRIVATE_PERMS)
+        set_file_mode(self._path, mode=_PRIVATE_PERMS)
         assert_private_file(self._path)
 
     def get(self, *keys, default=None) -> Union[dict, str, int]:

setup.py

@@ -47,6 +47,7 @@
           'xarray>=0.12.3',
           'pandas>0.20.0',
           'deprecated>=1.2.12',
+          'oschmod>=0.3.12; sys_platform == "win32"',
       ],
       extras_require={
           "dev": tests_require + [
@@ -55,6 +56,9 @@
               "sphinx-autodoc-typehints",
               "flake8",
               "myst-parser",
+          ],
+          "oschmod": [  # install oschmod even when platform is not Windows, e.g. for testing in CI.
+              "oschmod>=0.3.12"
           ]
       },
       entry_points={

tests/rest/auth/test_config.py

@@ -4,7 +4,7 @@
 import pytest
 
 import openeo.rest.auth.config
-from openeo.rest.auth.config import RefreshTokenStore, AuthConfig, PrivateJsonFile
+from openeo.rest.auth.config import RefreshTokenStore, AuthConfig, PrivateJsonFile, get_file_mode
 
 
 class TestPrivateJsonFile:
@@ -31,7 +31,7 @@ def test_permissions(self, tmp_path):
         assert not private.path.exists()
         private.set("foo", "bar", value=42)
         assert private.path.exists()
-        st_mode = private.path.stat().st_mode
+        st_mode = get_file_mode(private.path)
         assert st_mode & 0o777 == 0o600
 
     def test_wrong_permissions(self, tmp_path):
@@ -151,7 +151,7 @@ def test_public_file(self, tmp_path):
     def test_permissions(self, tmp_path):
         r = RefreshTokenStore(path=tmp_path)
         r.set_refresh_token("foo", "bar", "imd6$3cr3t")
-        st_mode = (tmp_path / RefreshTokenStore.DEFAULT_FILENAME).stat().st_mode
+        st_mode = get_file_mode(tmp_path / RefreshTokenStore.DEFAULT_FILENAME)
         assert st_mode & 0o777 == 0o600
 
     def test_get_set_refresh_token(self, tmp_path):

tests/test_config.py

@@ -1,7 +1,6 @@
 import contextlib
 import logging
 import os
-import platform
 import random
 import re
 import textwrap
@@ -168,6 +167,7 @@ def test_get_config_verbose(tmp_path, caplog, capsys, verbose, force_interactive
         config = get_config()
         assert config.get("general.verbose") == verbose
 
-    regex = re.compile(f"Loaded.*config from.*{config_path}")
+    # re.escape config_path because Windows paths contain "\"
+    regex = re.compile(f"Loaded.*config from.*{re.escape(str(config_path))}")
     assert regex.search(caplog.text)
     assert bool(regex.search(capsys.readouterr().out)) == on_stdout