Merge pull request #195 from ganwell/t/post_ca88

feat: add post-write-hook/callback

Author: Jean-Louis Fuchs

README.md

@@ -41,14 +41,13 @@ an unauthorized third party.
   refresh the lock. Consequently, we leverage this mechanism to also refresh the
   token.
 
-**pre_write_hook**: We provide an optional pre-write-hook feature that enhances
-the API's capabilities by adding a versioning system. The hook is invoked with
-the token as an argument **prior** to the document being saved using WebDAV-PUT.
-The API has the ability to verify this token and subsequently create a new
-version of the document. Once the hook operation is successfully completed,
-Manabi takes over and saves the document.
-
-A post-write-hook is not implemented, if you need one, please open an issue.
+**cb_hook_config**: We provide an optional write-hooks/callbacks feature that
+enhances the API's capabilities by adding a versioning system. The pre-write-
+hook/callback is invoked with the token as an argument **prior** to the document
+being saved using WebDAV-PUT. The API has the ability to verify this token and
+subsequently create a new version of the document. Once the hook operation is
+successfully completed, Manabi takes over and saves the document. Manabi then
+continues and invokes the post-write-hook/callback.
 
 ## Install
 
@@ -102,12 +101,20 @@ a computer, tokens should be expired by the time an adversary gets them.
 ```python
 from manabi import ManabiDAVApp
 
+# All hooks and callbacks are optional
+cb_hook_config = CallbackHookConfig(
+    pre_write_hook=_pre_write_hook,
+    pre_write_callback=_pre_write_callback,
+    post_write_hook=_post_write_hook,
+    post_write_callback=_post_write_callback,
+)
+
 postgres_dsn = "dbname=manabi user=manabi host=localhost password=manabi"
 config = {
     "mount_path": "/dav",
     "lock_storage": ManabiDbLockStorage(refresh, postgres_dsn),
     "provider_mapping": {
-        "/": ManabiProvider(settings.MEDIA_ROOT, pre_write_hook="http://127.0.0.1/hook"),
+        "/": ManabiProvider(settings.MEDIA_ROOT, cb_hook_config=cb_hook_config),
     },
     "middleware_stack": [
         WsgiDavDebugFilter,

manabi/conftest.py

@@ -66,21 +66,25 @@ def lock_storage(request, postgres_dsn):
 
 
 @pytest.fixture()
-def pre_write_hook():
+def write_hooks():
     try:
         mock._pre_write_hook = "http://127.0.0.1/pre_write_hook"
+        mock._post_write_hook = "http://127.0.0.1/post_write_hook"
         yield
     finally:
         mock._pre_write_hook = None
+        mock._post_write_hook = None
 
 
 @pytest.fixture()
-def pre_write_callback():
+def write_callback():
     try:
         mock._pre_write_callback = mock.check_token
+        mock._post_write_callback = mock.check_token
         yield mock.check_token
     finally:
         mock._pre_write_callback = None
+        mock._post_write_callback = None
 
 
 @pytest.fixture()

manabi/filesystem.py

@@ -1,12 +1,13 @@
 from pathlib import Path
 from typing import Any, Dict, Optional
 
+from attr import dataclass
 from wsgidav.dav_error import HTTP_FORBIDDEN, DAVError
 from wsgidav.fs_dav_provider import FileResource, FilesystemProvider, FolderResource
 
 from .token import Token
-from .type_alias import PreWriteType
-from .util import requests_session
+from .type_alias import WriteType
+from .util import cattrib, requests_session
 
 
 class ManabiFolderResource(FolderResource):
@@ -51,18 +52,26 @@ def set_last_modified(self, dest_path, time_stamp, dry_run):
         raise DAVError(HTTP_FORBIDDEN)
 
 
+@dataclass
+class CallbackHookConfig:
+    pre_write_hook: Optional[str] = cattrib(Optional[str], default=None)
+    pre_write_callback: Optional[WriteType] = cattrib(Optional[WriteType], default=None)
+    post_write_hook: Optional[str] = cattrib(Optional[str], default=None)
+    post_write_callback: Optional[WriteType] = cattrib(
+        Optional[WriteType], default=None
+    )
+
+
 class ManabiFileResource(FileResource):
     def __init__(
         self,
         path,
         environ,
         file_path,
         *,
-        pre_write_hook: Optional[str] = None,
-        pre_write_callback: Optional[PreWriteType] = None,
+        cb_hook_config: Optional[CallbackHookConfig] = None,
     ):
-        self._pre_write_hook = pre_write_hook
-        self._pre_write_callback = pre_write_callback
+        self._cb_config = cb_hook_config
         self._token = environ["manabi.token"]
         super().__init__(path, environ, file_path)
 
@@ -78,22 +87,46 @@ def support_recursive_move(self, dest_path):
     def move_recursive(self, dest_path):
         raise DAVError(HTTP_FORBIDDEN)
 
-    def begin_write(self, *, content_type):
+    def get_token_and_config(self):
         token = self._token
-        if token:
-            pre_hook = self._pre_write_hook
-            pre_callback = self._pre_write_callback
-
-            if pre_hook:
-                session = requests_session()
-                res = session.post(pre_hook, data=token.encode())
-                if res.status_code != 200:
-                    raise DAVError(HTTP_FORBIDDEN)
-            if pre_callback:
-                if not pre_callback(token):
-                    raise DAVError(HTTP_FORBIDDEN)
-            # The hook returned and hopefully created a new version.
-            # Now we can save.
+        config = self._cb_config
+        return token and config, token, config
+
+    def process_post_write_hooks(self):
+        ok, token, config = self.get_token_and_config()
+        if not ok:
+            return
+        post_hook = config.post_write_hook
+        post_callback = config.post_write_callback
+
+        if post_hook:
+            session = requests_session()
+            session.post(post_hook, data=token.encode())
+        if post_callback:
+            post_callback(token)
+
+    def end_write(self, *, with_errors):
+        if not with_errors:
+            self.process_post_write_hooks()
+
+    def process_pre_write_hooks(self):
+        ok, token, config = self.get_token_and_config()
+        if not ok:
+            return
+        pre_hook = config.pre_write_hook
+        pre_callback = config.pre_write_callback
+
+        if pre_hook:
+            session = requests_session()
+            res = session.post(pre_hook, data=token.encode())
+            if res.status_code != 200:
+                raise DAVError(HTTP_FORBIDDEN)
+        if pre_callback:
+            if not pre_callback(token):
+                raise DAVError(HTTP_FORBIDDEN)
+
+    def begin_write(self, *, content_type):
+        self.process_pre_write_hooks()
         return super().begin_write(content_type=content_type)
 
 
@@ -104,11 +137,9 @@ def __init__(
         *,
         readonly=False,
         shadow=None,
-        pre_write_hook: Optional[str] = None,
-        pre_write_callback: Optional[PreWriteType] = None,
+        cb_hook_config: Optional[CallbackHookConfig] = None,
     ):
-        self._pre_write_hook = pre_write_hook
-        self._pre_write_callback = pre_write_callback
+        self._cb_hook_config = cb_hook_config
         super().__init__(root_folder, readonly=readonly, shadow=shadow)
 
     def get_resource_inst(self, path: str, environ: Dict[str, Any]):
@@ -128,8 +159,7 @@ def get_resource_inst(self, path: str, environ: Dict[str, Any]):
                     path,
                     environ,
                     fp,
-                    pre_write_hook=self._pre_write_hook,
-                    pre_write_callback=self._pre_write_callback,
+                    cb_hook_config=self._cb_hook_config,
                 )
             else:
                 return None

manabi/filesystem_test.py

@@ -30,10 +30,12 @@ def test_get_and_put(tamper, expect_status, config: Dict[str, Any], server):
     ],
 )
 def test_get_and_put_hooked(
-    hook_status, expect_status, pre_write_hook, config: Dict[str, Any], server
+    hook_status, expect_status, write_hooks, config: Dict[str, Any], server
 ):
-    assert config["pre_write_hook"] == "http://127.0.0.1/pre_write_hook"
-    with mock.with_pre_write_hook(config, hook_status):
+    cb_hook_config = config["cb_hook_config"]
+    assert cb_hook_config.pre_write_hook == "http://127.0.0.1/pre_write_hook"
+    assert cb_hook_config.post_write_hook == "http://127.0.0.1/post_write_hook"
+    with mock.with_write_hooks(config, hook_status):
         req = mock.make_req(config)
         res = requests.get(req)
         assert res.status_code == 200
@@ -45,11 +47,12 @@ def test_get_and_put_hooked(
 def test_get_and_put_called(
     callback_return,
     expect_status,
-    pre_write_callback,
+    write_callback,
     config: Dict[str, Any],
     server,
 ):
-    assert config["pre_write_callback"] == pre_write_callback
+    assert config["cb_hook_config"].pre_write_callback == write_callback
+    assert config["cb_hook_config"].post_write_callback == write_callback
     req = mock.make_req(config)
     res = requests.get(req)
     assert res.status_code == 200

manabi/mock.py

@@ -18,11 +18,11 @@
 
 from . import ManabiDAVApp, lock as mlock
 from .auth import ManabiAuthenticator
-from .filesystem import ManabiProvider
+from .filesystem import CallbackHookConfig, ManabiProvider
 from .lock import ManabiDbLockStorage as ManabiDbLockStorageOrig
 from .log import HeaderLogger, ResponseLogger
 from .token import Config, Key, Token, now
-from .type_alias import PreWriteType
+from .type_alias import WriteType
 from .util import get_rfc1123_time
 
 _servers: Dict[Tuple[str, int], wsgi.Server] = dict()
@@ -58,11 +58,12 @@ def check_token(token: Token) -> bool:
 
 
 _pre_write_hook: Optional[str] = None
+_post_write_hook: Optional[str] = None
 
 
 @contextmanager
-def with_pre_write_hook(config: Dict[str, Any], status_code=None):
-    if not _pre_write_hook:
+def with_write_hooks(config: Dict[str, Any], status_code=None):
+    if not (_pre_write_hook and _post_write_hook):
         return
 
     cfg = Config.from_dictionary(config)
@@ -79,10 +80,12 @@ def check_token(request, context):
 
     with requests_mock.Mocker(real_http=True) as m:
         m.post(_pre_write_hook, text=check_token)
+        m.post(_post_write_hook, text=check_token)
         yield
 
 
-_pre_write_callback: Optional[PreWriteType] = None
+_pre_write_callback: Optional[WriteType] = None
+_post_write_callback: Optional[WriteType] = None
 
 
 def get_config(server_dir: Path, lock_storage: Union[Path, str]):
@@ -93,19 +96,20 @@ def get_config(server_dir: Path, lock_storage: Union[Path, str]):
         lock_obj = mlock.ManabiShelfLockLockStorage(refresh, lock_storage)
     else:
         lock_obj = mlock.ManabiDbLockStorage(refresh, lock_storage)
+    cb_hook_config = CallbackHookConfig(
+        pre_write_hook=_pre_write_hook,
+        pre_write_callback=_pre_write_callback,
+        post_write_hook=_post_write_hook,
+        post_write_callback=_post_write_callback,
+    )
     return {
-        "pre_write_hook": _pre_write_hook,
-        "pre_write_callback": _pre_write_callback,
+        "cb_hook_config": cb_hook_config,
         "host": "0.0.0.0",
         "port": 8081,
         "mount_path": "/dav",
         "lock_storage": lock_obj,
         "provider_mapping": {
-            "/": ManabiProvider(
-                server_dir,
-                pre_write_hook=_pre_write_hook,
-                pre_write_callback=_pre_write_callback,
-            ),
+            "/": ManabiProvider(server_dir, cb_hook_config=cb_hook_config),
         },
         "middleware_stack": [
             HeaderLogger,

manabi/type_alias.py

@@ -21,4 +21,4 @@
     bool,
 ]
 OptionalProp = Optional[PropType]
-PreWriteType = Callable[["Token"], bool]
+WriteType = Callable[["Token"], bool]