BE: Refactor sync endpoint to accept JSON payloads and update related tests #1652

front/plugins/sync/sync.py

@@ -298,7 +298,7 @@ def send_data(api_token, file_content, encryption_key, file_path, node_name, pre
         final_endpoint = hub_url + endpoint
 
         try:
-            response = requests.post(final_endpoint, data=data, headers=headers, timeout=5)
+            response = requests.post(final_endpoint, json=data, headers=headers, timeout=5)
             mylog('verbose', [f'[{pluginName}] Tried endpoint: {final_endpoint}, status: {response.status_code}'])
 
             if response.status_code == 200:

server/api_server/openapi/schemas.py

@@ -893,9 +893,10 @@ class CreateNotificationRequest(BaseModel):
 
 class SyncPushRequest(BaseModel):
     """Request to push data to sync."""
-    data: dict = Field(..., description="Data to sync")
+    data: str = Field(..., description="Encrypted data payload (ciphertext string)")
     node_name: str = Field(..., description="Name of the node sending data")
     plugin: str = Field(..., description="Plugin identifier")
+    file_path: Optional[str] = Field(None, description="Source file path on the node")
 
 
 class SyncPullResponse(BaseResponse):

server/api_server/sync_endpoint.py

@@ -47,9 +47,10 @@ def handle_sync_get():
 
 def handle_sync_post():
     """Handle POST requests for SYNC (HUB receiving from NODE)."""
-    data = request.form.get("data", "")
-    node_name = request.form.get("node_name", "")
-    plugin = request.form.get("plugin", "")
+    body = request.get_json(silent=True) or {}
+    data = body.get("data", "")
+    node_name = body.get("node_name", "")
+    plugin = body.get("plugin", "")
 
     storage_path = INSTALL_PATH + "/log/plugins"
     os.makedirs(storage_path, exist_ok=True)

test/api_endpoints/test_sync_endpoint.py

@@ -0,0 +1,124 @@
+"""Tests for the /sync POST and GET endpoints.
+
+Covers:
+  - Authentication enforcement (403 on missing/invalid token)
+  - Content-type enforcement on POST (regression for data= vs json= bug)
+  - Happy-path POST returns 200
+  - GET auth enforcement
+"""
+
+import os
+import sys
+from unittest.mock import patch
+
+import pytest
+
+INSTALL_PATH = os.getenv("NETALERTX_APP", "/app")
+sys.path.extend([f"{INSTALL_PATH}/front/plugins", f"{INSTALL_PATH}/server"])
+
+from helper import get_setting_value  # noqa: E402
+from api_server.api_server_start import app  # noqa: E402
+import api_server.sync_endpoint as sync_endpoint  # noqa: E402
+
+
+@pytest.fixture(scope="session")
+def api_token():
+    """Load API token from system settings."""
+    return get_setting_value("API_TOKEN")
+
+
+@pytest.fixture
+def client():
+    """Flask test client."""
+    with app.test_client() as client:
+        yield client
+
+
+def auth_headers(token):
+    """Helper to construct Authorization header."""
+    return {"Authorization": f"Bearer {token}"}
+
+
+# ========================================================================
+# POST /sync – authentication
+# ========================================================================
+
+def test_sync_post_no_token_is_forbidden(client):
+    resp = client.post("/sync")
+    assert resp.status_code == 403
+
+
+def test_sync_post_invalid_token_is_forbidden(client):
+    resp = client.post("/sync", headers=auth_headers("INVALID-TOKEN"))
+    assert resp.status_code == 403
+
+
+# ========================================================================
+# POST /sync – content-type enforcement
+# Regression: node used to send data= (form-encoded); validation rejects it.
+# ========================================================================
+
+def test_sync_post_form_encoded_returns_415(client, api_token):
+    """Form-encoded body must be rejected with 415 Unsupported Media Type.
+
+    Regression test: before the fix sync.py used ``requests.post(data=…)``
+    which sends application/x-www-form-urlencoded. The validate_request
+    middleware requires application/json — this test ensures that contract
+    is enforced so the node can never silently regress to form encoding.
+    """
+    resp = client.post(
+        "/sync",
+        headers=auth_headers(api_token),
+        data={"data": "payload", "plugin": "ARPSCAN", "node_name": "Node1"},
+        content_type="application/x-www-form-urlencoded",
+    )
+    assert resp.status_code == 415
+
+
+def test_sync_post_json_body_is_accepted(client, api_token, tmp_path):
+    """JSON body must pass validation and return 200."""
+    plugins_dir = tmp_path / "log" / "plugins"
+    plugins_dir.mkdir(parents=True)
+
+    with patch.object(sync_endpoint, "INSTALL_PATH", str(tmp_path)):
+        resp = client.post(
+            "/sync",
+            headers=auth_headers(api_token),
+            json={"data": "test_payload", "plugin": "TESTPLUGIN", "node_name": "TestNode"},
+        )
+
+    assert resp.status_code == 200
+    data = resp.get_json()
+    assert data is not None
+    assert "message" in data
+
+
+def test_sync_post_json_body_writes_encoded_file(client, api_token, tmp_path):
+    """A successful POST must persist an encoded file in the plugins log dir."""
+    plugins_dir = tmp_path / "log" / "plugins"
+    plugins_dir.mkdir(parents=True)
+
+    with patch.object(sync_endpoint, "INSTALL_PATH", str(tmp_path)):
+        client.post(
+            "/sync",
+            headers=auth_headers(api_token),
+            json={"data": "encrypted_blob", "plugin": "ARPSCAN", "node_name": "Node1"},
+        )
+
+    written = list(plugins_dir.glob("last_result.ARPSCAN.encoded.Node1.*.log"))
+    assert len(written) == 1
+    assert written[0].read_text() == "encrypted_blob"
+
+
+# ========================================================================
+# GET /sync – authentication
+# ========================================================================
+
+def test_sync_get_no_token_is_forbidden(client):
+    resp = client.get("/sync")
+    assert resp.status_code == 403
+
+
+def test_sync_get_invalid_token_is_forbidden(client):
+    resp = client.get("/sync", headers=auth_headers("INVALID-TOKEN"))
+    assert resp.status_code == 403

test/plugins/test_sync_protocol.py

@@ -46,7 +46,7 @@ def _send_data(api_token, file_content, encryption_key, file_path, node_name, pr
     }
     headers = {"Authorization": f"Bearer {api_token}"}
     try:
-        response = requests.post(hub_url + API_ENDPOINT, data=data, headers=headers, timeout=5)
+        response = requests.post(hub_url + API_ENDPOINT, json=data, headers=headers, timeout=5)
         return response.status_code == 200
     except requests.RequestException:
         return False
@@ -73,6 +73,16 @@ def _node_name_from_filename(file_name: str) -> str:
     return parts[2] if ("decoded" in file_name or "encoded" in file_name) else parts[1]
 
 
+def _should_delete_after_process(filename: str) -> bool:
+    """Mirror of the delete-after-process condition in execute_plugin() (server/plugin.py).
+
+    Only node-sync intermediary files (.encoded. / .decoded.) are removed after
+    processing.  Local plugin result files (last_result.ARPSCAN.log etc.) must
+    survive so SYNC Mode 1 can read and forward them to the hub.
+    """
+    return ".encoded." in filename or ".decoded." in filename
+
+
 def _determine_mode(hub_url: str, send_devices: bool, plugins_to_sync: list, pull_nodes: list):
     """Mirror of the is_hub / is_node detection block in sync.main()."""
     is_node = len(hub_url) > 0 and (send_devices or bool(plugins_to_sync))
@@ -205,7 +215,7 @@ class TestSendData:
         with patch("requests.post", return_value=resp) as mock_post:
             _send_data(API_TOKEN, '{"data":[]}', ENCRYPTION_KEY,
                        "/tmp/file.log", "node1", "SYNC", HUB_URL)
-        payload = mock_post.call_args[1]["data"]
+        payload = mock_post.call_args[1]["json"]
         assert "data" in payload          # encrypted blob
         assert payload["file_path"] == "/tmp/file.log"
         assert payload["plugin"] == "SYNC"
@@ -219,7 +229,7 @@ class TestSendData:
         with patch("requests.post", return_value=resp) as mock_post:
             _send_data(API_TOKEN, plaintext, ENCRYPTION_KEY,
                        "/tmp/file.log", "node1", "SYNC", HUB_URL)
-        transmitted = mock_post.call_args[1]["data"]["data"]
+        transmitted = mock_post.call_args[1]["json"]["data"]
         assert transmitted != plaintext
         # Verify it round-trips correctly
         assert decrypt_data(transmitted, ENCRYPTION_KEY) == plaintext
@@ -409,5 +419,49 @@ class TestReceiveInsert:
         inserted = sync_insert_devices(conn, [device], existing_macs=set())
         assert inserted == 1
 
+
+# ===========================================================================
+# Plugin result file retention (regression for execute_plugin delete bug)
+# ===========================================================================
+
+class TestPluginFileRetention:
+    """Regression for the execute_plugin() delete-condition bug (server/plugin.py).
+
+    Before the fix the condition was ``filename != "last_result.log"``.  No
+    plugin ever writes to that literal name — all write ``last_result.ARPSCAN.log``
+    etc. — so every local result file was deleted immediately after processing,
+    before SYNC Mode 1 had a chance to read and forward it to the hub.
+
+    The corrected condition deletes ONLY ``.encoded.`` / ``.decoded.``
+    node-sync intermediary files.  Local plugin result files must survive.
+    """
+
+    def test_local_result_file_not_flagged_for_deletion(self):
+        assert _should_delete_after_process("last_result.ARPSCAN.log") is False
+
+    def test_local_result_files_for_common_plugins_not_flagged(self):
+        for plugin in ("NMAP", "PIHOLE", "SYNC", "DHCPLEASES", "ARPSCAN"):
+            fname = f"last_result.{plugin}.log"
+            assert _should_delete_after_process(fname) is False, \
+                f"{fname} must NOT be deleted — SYNC Mode 1 still needs it"
+
+    def test_encoded_node_sync_file_flagged_for_deletion(self):
+        assert _should_delete_after_process("last_result.ARPSCAN.encoded.Node1.1.log") is True
+
+    def test_decoded_node_sync_file_flagged_for_deletion(self):
+        assert _should_delete_after_process("last_result.ARPSCAN.decoded.Node1.1.log") is True
+
+    def test_encoded_files_with_various_node_names_flagged(self):
+        for node in ("Node1", "Home_Hub", "Site_B", "OfficeNode"):
+            fname = f"last_result.ARPSCAN.encoded.{node}.1.log"
+            assert _should_delete_after_process(fname) is True, \
+                f"{fname} should be deleted after processing"
+
+    def test_decoded_files_with_various_node_names_flagged(self):
+        for node in ("Node1", "Home_Hub", "Site_B"):
+            fname = f"last_result.ARPSCAN.decoded.{node}.2.log"
+            assert _should_delete_after_process(fname) is True, \
+                f"{fname} should be deleted after processing"
+
     def test_empty_device_list_returns_zero(self, conn):
         assert sync_insert_devices(conn, [], existing_macs=set()) == 0