You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ponymail.apache.org by se...@apache.org on 2022/01/31 01:15:24 UTC

[incubator-ponymail-foal] branch master updated (19a0efb -> 930e75a)

This is an automated email from the ASF dual-hosted git repository.

sebb pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git.


    from 19a0efb  Show source id
     new bd3e01e  mgmt API updates:
     new 930e75a  Bump server version

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 server/endpoints/mgmt.py  | 117 ++++++++++++++++++++++++++++------------------
 server/server_version.py  |   2 +-
 test/itest_integration.py |  26 ++++++-----
 3 files changed, 88 insertions(+), 57 deletions(-)

[incubator-ponymail-foal] 02/02: Bump server version

Posted by se...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sebb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git

commit 930e75a3af21d621f8bc501b20466c803b14e4a2
Author: Sebb <se...@apache.org>
AuthorDate: Mon Jan 31 01:15:14 2022 +0000

    Bump server version
---
 server/server_version.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/server_version.py b/server/server_version.py
index b0df0f9..5a96f7e 100644
--- a/server/server_version.py
+++ b/server/server_version.py
@@ -1,2 +1,2 @@
 # This file is generated by server/update_version.sh
-PONYMAIL_SERVER_VERSION = '9a4ceb7'
+PONYMAIL_SERVER_VERSION = 'bd3e01e'

[incubator-ponymail-foal] 01/02: mgmt API updates:

Posted by se...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sebb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-ponymail-foal.git

commit bd3e01e7bb155ee715eb10aa01148bdc9aabfca1
Author: Sebb <se...@apache.org>
AuthorDate: Mon Jan 31 01:14:47 2022 +0000

    mgmt API updates:
    
    - list id validation (fixes #221)
    - allow optional fields (fixes #222)
---
 server/endpoints/mgmt.py  | 117 ++++++++++++++++++++++++++++------------------
 test/itest_integration.py |  26 ++++++-----
 2 files changed, 87 insertions(+), 56 deletions(-)

diff --git a/server/endpoints/mgmt.py b/server/endpoints/mgmt.py
index 9c663bc..451c941 100644
--- a/server/endpoints/mgmt.py
+++ b/server/endpoints/mgmt.py
@@ -21,6 +21,7 @@ import plugins.server
 import plugins.session
 import plugins.messages
 import plugins.auditlog
+import re
 import typing
 import aiohttp.web
 
@@ -28,6 +29,7 @@ import aiohttp.web
 # This is was done to make testing easier.
 # There are very few such changes so this should not affect performance unduly
 
+LISTID_RE = re.compile(r"\A<?[-_a-z0-9]+[.@][-_a-z0-9.]+>?\Z")
 
 async def process(
     server: plugins.server.BaseServer, session: plugins.session.SessionObject, indata: dict,
@@ -138,71 +140,96 @@ async def process(
         new_from = indata.get("from")
         new_subject = indata.get("subject")
         new_list = indata.get("list")
-        private = indata.get("private", "yes") == "yes" # Assume private unless notified otherwise
         new_body = indata.get("body")
         attach_edit = indata.get("attachments", None)
 
         # Check for consistency so we don't pollute the database
         if not isinstance(doc, str):
             raise ValueError("Document ID is missing or invalid")
-        if not isinstance(new_from, str):
+        # Allow for omitted values
+        if new_from and not isinstance(new_from, str):
             raise ValueError("Author field must be a text string!")
-        if not isinstance(new_subject, str):
+        if new_subject and not isinstance(new_subject, str):
             raise ValueError("Subject field must be a text string!")
-        if not isinstance(new_list, str):
+        if new_list and not isinstance(new_list, str):
             raise ValueError("List ID field must be a text string!")
-        if not isinstance(new_body, str):
+        if new_list and not re.match(LISTID_RE, new_list):
+            raise ValueError("List ID field must match listname[@.]domain !")
+        if new_body and not isinstance(new_body, str):
             raise ValueError("Email body must be a text string!")
 
-        # Convert List-ID after verification
-        lid = "<" + new_list.strip("<>").replace("@", ".") + ">"  # foo@bar.baz -> <foo.bar.baz>
-
         email = await plugins.messages.get_email(session, permalink=doc)
         if email:
-            # Test if only privacy may have changed
-            privacy_only = (
-                    attach_edit is None and
-                    email["from"] == new_from and
-                    email["subject"] == new_subject and
-                    email["list"] == lid and
-                    email["body"] == new_body
-            )
-            email["from_raw"] = new_from
-            email["from"] = new_from
-            email["subject"] = new_subject
-            email["private"] = private
+
+            new_private = indata.get("private", True) # This allows it to be omitted; assume private
+             # the property could also be a string, in which case look for explicit public value
+            if not isinstance(new_private, bool):
+                new_private = (new_private != 'no') # True unless value is 'no', i.e. public
+            # if property is absent, we want to set it, so don't default it
+            changed_private = (email.get("private") != new_private)
+            if changed_private:
+                email["private"] = new_private # this does not require the source to be hidden
+
+            hide_source = False # we hide the source if any of its derived fields are changed
+
+            # have any derived fields changed?
+            if new_from and not email["from"] == new_from:
+                email["from_raw"] = new_from
+                email["from"] = new_from
+                hide_source = True
+
+            if new_subject and not email["subject"] == new_subject:
+                email["subject"] = new_subject
+                hide_source = True
+    
             origin_lid = email["list_raw"]
-            email["list"] = lid
-            email["list_raw"] = lid
-            email["forum"] = lid.strip("<>").replace(".", "@", 1)
-            email["body"] = new_body
-            email["body_short"] = new_body[:plugins.messages.SHORT_BODY_MAX_LEN+1]
+            new_lid = origin_lid # needed for audit log
+            if new_list:
+                # Convert List-ID after verification
+                new_lid = "<" + new_list.strip("<>").replace("@", ".") + ">"  # foo@bar.baz -> <foo.bar.baz>
+                if not new_lid == origin_lid:
+                    email["list"] = new_lid
+                    email["list_raw"] = new_lid
+                    email["forum"] = new_lid.strip("<>").replace(".", "@", 1)
+                    hide_source = True
+
+            if new_body and not email["body"] == new_body:
+                email["body"] = new_body
+                email["body_short"] = new_body[:plugins.messages.SHORT_BODY_MAX_LEN+1]
+                hide_source = True
+
             if attach_edit is not None:  # Only set if truly editing attachments...
                 email["attachments"] = attach_edit
+                hide_source = True
 
             # Save edited email
-            if "id" in email: # id is not a valid property for mbox
-                del email["id"]
-            await session.database.update(
-                index=session.database.dbs.db_mbox, body={"doc": email}, id=email["mid"], refresh='wait_for',
-            )
-
-            # Fetch source, mark as deleted (modified) and save IF anything but just privacy changed
-            # We do this, as we can't edit the source easily, so we mark it as off-limits instead.
-            if not privacy_only:
-                source = await plugins.messages.get_source(session, permalink=email["dbid"], raw=True)
-                if source:
-                    docid = source["_id"]
-                    source = source["_source"]
-                    source["deleted"] = True
-                    await session.database.update(
-                        index=session.database.dbs.db_source, body={"doc": source}, id=docid, refresh='wait_for',
-                    )
+            if changed_private or hide_source: # something changed
+                if "id" in email: # id is not a valid property for mbox
+                    del email["id"]
+                await session.database.update(
+                    index=session.database.dbs.db_mbox, body={"doc": email}, id=email["mid"], refresh='wait_for',
+                )
 
-            await plugins.auditlog.add_entry(session, action="edit", target=doc, lid=lid,
-                                             log= f"Edited email {doc} from {origin_lid} archives ({origin_lid} -> {lid})")
+                # Fetch source, mark as deleted (modified) and save IF anything but just privacy changed
+                # We do this, as we can't edit the source easily, so we mark it as off-limits instead.
+                if hide_source:
+                    source = await plugins.messages.get_source(session, permalink=email["dbid"], raw=True)
+                    if source:
+                        docid = source["_id"]
+                        source = source["_source"]
+                        source["deleted"] = True
+                        await session.database.update(
+                            index=session.database.dbs.db_source, body={"doc": source}, id=docid, refresh='wait_for',
+                        )
+
+                # TODO this should perhaps show the actual changes?
+                await plugins.auditlog.add_entry(session, action="edit", target=doc, lid=new_lid,
+                                             log= f"Edited email {doc} from {origin_lid} archives ({origin_lid} -> {new_lid})")
+
+                return aiohttp.web.Response(headers={}, status=200, text="Email successfully saved")
+            else:
+                raise ValueError("No changes made!")
 
-            return aiohttp.web.Response(headers={}, status=200, text="Email successfully saved")
         return aiohttp.web.Response(headers={}, status=404, text="Email not found!")
 
     return aiohttp.web.Response(headers={}, status=404, text="Unknown mgmt command requested")
diff --git a/test/itest_integration.py b/test/itest_integration.py
index 010e11d..b50069c 100644
--- a/test/itest_integration.py
+++ b/test/itest_integration.py
@@ -187,12 +187,12 @@ def test_private_stats():
         check_access(email, cookies)
 
 def mgmt_get_text(params, cookies, expected=200):
-    res = requests.post(f"{API_BASE}/mgmt.lua", params=params, cookies=cookies)
+    res = requests.post(f"{API_BASE}/mgmt.json", json=params, cookies=cookies)
     assert res.status_code == expected, res.text
     return res.text
 
 def mgmt_get_json(params, cookies, expected=200):
-    res = requests.post(f"{API_BASE}/mgmt.lua", params=params, cookies=cookies)
+    res = requests.post(f"{API_BASE}/mgmt.json", json=params, cookies=cookies)
     assert res.status_code == expected, res.text
     return res.json()
 
@@ -217,23 +217,26 @@ def test_mgmt_validation():
     text = mgmt_get_text({"action": 'edit'}, admin_cookies, 500)
     assert "ValueError: Document ID is missing or invalid" in text
 
-    text = mgmt_get_text({"action": 'edit', "document": '1234'}, admin_cookies, 500)
+    text = mgmt_get_text({"action": 'edit', "document": None}, admin_cookies, 500)
+    assert "ValueError: Document ID is missing or invalid" in text
+
+    text = mgmt_get_text({"action": 'edit', "document": 'abcd', "from": 1234}, admin_cookies, 500)
     assert "ValueError: Author field" in text
 
-    text = mgmt_get_text({"action": 'edit', "document": '1234', "from": 'sender'}, admin_cookies, 500)
+    text = mgmt_get_text({"action": 'edit', "document": 'abcd', "subject": 1234}, admin_cookies, 500)
     assert "ValueError: Subject field" in text
 
-    text = mgmt_get_text({"action": 'edit', "document": '1234', "from": 'sender', "subject": 'Test Email'}, admin_cookies, 500)
+    text = mgmt_get_text({"action": 'edit', "document": 'abcd', "list": True}, admin_cookies, 500)
     assert "ValueError: List ID field" in text
 
+    text = mgmt_get_text({"action": 'edit', "document": 'abcd', "list": "True"}, admin_cookies, 500)
+    assert "ValueError: List ID field must match" in text
+
     text = mgmt_get_text(
-        {"action": 'edit', "document": '1234', "from": 'sender', "subject": 'Test Email', "list": 'abc'},
-        admin_cookies, 500)
+        {"action": 'edit', "document": 'abcd', "body": 1234}, admin_cookies, 500)
     assert "ValueError: Email body" in text
 
-    text = mgmt_get_text(
-        {"action": 'edit', "document": '1234', "from": 'sender', "subject": 'Test Email', "list": 'abc', "body": 'body'},
-        admin_cookies, 404)
+    text = mgmt_get_text({"action": 'edit', "document": 'abcd'}, admin_cookies, 404)
     assert "Email not found!" in text
 
 def test_mgmt_log_before():
@@ -305,7 +308,8 @@ def test_mgmt_edit():
     text = mgmt_get_text(
         {
             "action": 'edit', "document": DOCUMENT_EDIT_TEST,
-            "from": '', "subject": '', "list": test_list_id, "body": time.time(), "private": False,
+            # "from": '', "subject": '', "list": test_list_id, 
+            "body": str(time.time()), "private": False,
         },
         admin_cookies
         )