From 5910a7c2a1ee742033f7436bd57c37acf89a2f2e Mon Sep 17 00:00:00 2001 From: "Jokob @NetAlertX" <96159884+jokob-sk@users.noreply.github.com> Date: Sun, 24 May 2026 01:11:00 +0000 Subject: [PATCH] BE: Refactor sync endpoint to accept JSON payloads and update related tests #1652 --- front/plugins/sync/sync.py | 2 +- server/api_server/openapi/schemas.py | 3 +- server/api_server/sync_endpoint.py | 7 +- test/api_endpoints/test_sync_endpoint.py | 124 +++++++++++++++++++++++ test/plugins/test_sync_protocol.py | 60 ++++++++++- 5 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 test/api_endpoints/test_sync_endpoint.py diff --git a/front/plugins/sync/sync.py b/front/plugins/sync/sync.py index 514659b3..7accd5d0 100755 --- a/front/plugins/sync/sync.py +++ b/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: diff --git a/server/api_server/openapi/schemas.py b/server/api_server/openapi/schemas.py index 66693901..3b1728d7 100644 --- a/server/api_server/openapi/schemas.py +++ b/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): diff --git a/server/api_server/sync_endpoint.py b/server/api_server/sync_endpoint.py index 7556041d..fe086a2a 100755 --- a/server/api_server/sync_endpoint.py +++ b/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) diff --git a/test/api_endpoints/test_sync_endpoint.py b/test/api_endpoints/test_sync_endpoint.py new file mode 100644 index 00000000..cab43e07 --- /dev/null +++ b/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 diff --git a/test/plugins/test_sync_protocol.py b/test/plugins/test_sync_protocol.py index 9d46cf26..8958c8ce 100644 --- a/test/plugins/test_sync_protocol.py +++ b/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