From 20d9d9651a9b0def70bb53187c5b1206823ed8f6 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Wed, 15 Jul 2026 15:30:06 +0200 Subject: [PATCH] fix(freeradius): return the created entry's remote id from create_radius_* add_client/add_user's response carries no id, so create_radius_client() and create_radius_user() now look the new entry up via get_radius_clients()/get_radius_users() (matched by name/username) immediately after creation. Callers need this id to address the entry in later set_*/del_* calls -- without it there was no way to store a reference to what was just created. --- napalm_opnsense/opnsense.py | 14 +++++++++++--- tests/unit/test_driver.py | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/napalm_opnsense/opnsense.py b/napalm_opnsense/opnsense.py index a2c087f..6cf9f62 100644 --- a/napalm_opnsense/opnsense.py +++ b/napalm_opnsense/opnsense.py @@ -1144,13 +1144,17 @@ class OPNsenseDriver(FirewallDriver): Calls ``POST /api/freeradius/client/add_client``, then ``POST /api/freeradius/service/reconfigure`` to apply -- a saved client has no effect on the running radiusd until reconfigured. + The add response carries no id, so the new client's uuid is looked + up afterwards via get_radius_clients() (matched by name) -- callers + need it to address this client in later set_client/del_client calls. """ payload = {"client": {"name": name, "ip": ip, "secret": secret}} result = self._post("/api/freeradius/client/add_client", payload) if result.get("result") != "saved": return {"success": False, "validations": result.get("validations", {})} self._post("/api/freeradius/service/reconfigure") - return {"success": True} + created = next((c for c in self.get_radius_clients() if c["name"] == name), None) + return {"success": True, "id": created["id"] if created else None} def delete_radius_client(self, client_id: str) -> dict[str, Any]: """Delete a FreeRADIUS NAS client by uuid and apply the change.""" @@ -1184,14 +1188,18 @@ class OPNsenseDriver(FirewallDriver): """Create a FreeRADIUS user and apply the change. Calls ``POST /api/freeradius/user/add_user``, then - ``POST /api/freeradius/service/reconfigure`` to apply. + ``POST /api/freeradius/service/reconfigure`` to apply. The add + response carries no id, so the new user's uuid is looked up + afterwards via get_radius_users() (matched by username) -- callers + need it to address this user in later set_user/del_user calls. """ payload = {"user": {"username": username, "password": password}} result = self._post("/api/freeradius/user/add_user", payload) if result.get("result") != "saved": return {"success": False, "validations": result.get("validations", {})} self._post("/api/freeradius/service/reconfigure") - return {"success": True} + created = next((u for u in self.get_radius_users() if u["username"] == username), None) + return {"success": True, "id": created["id"] if created else None} def delete_radius_user(self, user_id: str) -> dict[str, Any]: """Delete a FreeRADIUS user by uuid and apply the change.""" diff --git a/tests/unit/test_driver.py b/tests/unit/test_driver.py index 9373b7a..0c8f9cb 100644 --- a/tests/unit/test_driver.py +++ b/tests/unit/test_driver.py @@ -1175,11 +1175,17 @@ class TestGetRadiusClients: class TestCreateRadiusClient: - def test_success_reconfigures_service(self, driver): + def test_success_returns_remote_id(self, driver): + driver._post = lambda path, data=None: {"result": "saved"} + driver._get = lambda path: RADIUS_CLIENTS_RESPONSE + result = driver.create_radius_client("ap-lobby", "10.0.0.5/32", "s3cr3t") + assert result == {"success": True, "id": "e5483f1b-936b-47ba-8cca-24d56ad643c6"} + + def test_success_calls_add_then_reconfigure(self, driver): calls = [] driver._post = lambda path, data=None: calls.append((path, data)) or {"result": "saved"} - result = driver.create_radius_client("ap-lobby", "10.0.0.5/32", "s3cr3t") - assert result == {"success": True} + driver._get = lambda path: RADIUS_CLIENTS_RESPONSE + driver.create_radius_client("ap-lobby", "10.0.0.5/32", "s3cr3t") assert calls[0] == ( "/api/freeradius/client/add_client", {"client": {"name": "ap-lobby", "ip": "10.0.0.5/32", "secret": "s3cr3t"}}, @@ -1197,6 +1203,12 @@ class TestCreateRadiusClient: assert result["validations"] == {"client.name": "A value is required."} assert calls == ["/api/freeradius/client/add_client"] + def test_created_entry_not_found_after_add_returns_no_id(self, driver): + driver._post = lambda path, data=None: {"result": "saved"} + driver._get = lambda path: {"rows": []} + result = driver.create_radius_client("ap-lobby", "10.0.0.5/32", "s3cr3t") + assert result == {"success": True, "id": None} + class TestDeleteRadiusClient: def test_success_reconfigures_service(self, driver): @@ -1246,11 +1258,17 @@ class TestGetRadiusUsers: class TestCreateRadiusUser: - def test_success_reconfigures_service(self, driver): + def test_success_returns_remote_id(self, driver): + driver._post = lambda path, data=None: {"result": "saved"} + driver._get = lambda path: RADIUS_USERS_RESPONSE + result = driver.create_radius_user("jdoe", "hunter2") + assert result == {"success": True, "id": "71da23fd-03f3-4e1f-a7e1-bb25645b981e"} + + def test_success_calls_add_then_reconfigure(self, driver): calls = [] driver._post = lambda path, data=None: calls.append((path, data)) or {"result": "saved"} - result = driver.create_radius_user("jdoe", "hunter2") - assert result == {"success": True} + driver._get = lambda path: RADIUS_USERS_RESPONSE + driver.create_radius_user("jdoe", "hunter2") assert calls[0] == ( "/api/freeradius/user/add_user", {"user": {"username": "jdoe", "password": "hunter2"}}, @@ -1267,6 +1285,12 @@ class TestCreateRadiusUser: assert result["success"] is False assert calls == ["/api/freeradius/user/add_user"] + def test_created_entry_not_found_after_add_returns_no_id(self, driver): + driver._post = lambda path, data=None: {"result": "saved"} + driver._get = lambda path: {"rows": []} + result = driver.create_radius_user("jdoe", "hunter2") + assert result == {"success": True, "id": None} + class TestDeleteRadiusUser: def test_success_reconfigures_service(self, driver):