fix(freeradius): return the created entry's remote id from create_radius_*
CI / test (3.10) (push) Failing after 8s
CI / test (3.11) (push) Failing after 7s
CI / test (3.12) (push) Failing after 7s
CI / test (3.9) (push) Failing after 7s

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.
This commit is contained in:
Christian Manivong
2026-07-15 15:30:06 +02:00
parent e51607a020
commit 20d9d9651a
2 changed files with 41 additions and 9 deletions
+11 -3
View File
@@ -1144,13 +1144,17 @@ class OPNsenseDriver(FirewallDriver):
Calls ``POST /api/freeradius/client/add_client``, then Calls ``POST /api/freeradius/client/add_client``, then
``POST /api/freeradius/service/reconfigure`` to apply -- a saved ``POST /api/freeradius/service/reconfigure`` to apply -- a saved
client has no effect on the running radiusd until reconfigured. 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}} payload = {"client": {"name": name, "ip": ip, "secret": secret}}
result = self._post("/api/freeradius/client/add_client", payload) result = self._post("/api/freeradius/client/add_client", payload)
if result.get("result") != "saved": if result.get("result") != "saved":
return {"success": False, "validations": result.get("validations", {})} return {"success": False, "validations": result.get("validations", {})}
self._post("/api/freeradius/service/reconfigure") 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]: def delete_radius_client(self, client_id: str) -> dict[str, Any]:
"""Delete a FreeRADIUS NAS client by uuid and apply the change.""" """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. """Create a FreeRADIUS user and apply the change.
Calls ``POST /api/freeradius/user/add_user``, then 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}} payload = {"user": {"username": username, "password": password}}
result = self._post("/api/freeradius/user/add_user", payload) result = self._post("/api/freeradius/user/add_user", payload)
if result.get("result") != "saved": if result.get("result") != "saved":
return {"success": False, "validations": result.get("validations", {})} return {"success": False, "validations": result.get("validations", {})}
self._post("/api/freeradius/service/reconfigure") 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]: def delete_radius_user(self, user_id: str) -> dict[str, Any]:
"""Delete a FreeRADIUS user by uuid and apply the change.""" """Delete a FreeRADIUS user by uuid and apply the change."""
+30 -6
View File
@@ -1175,11 +1175,17 @@ class TestGetRadiusClients:
class TestCreateRadiusClient: 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 = [] calls = []
driver._post = lambda path, data=None: calls.append((path, data)) or {"result": "saved"} 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") driver._get = lambda path: RADIUS_CLIENTS_RESPONSE
assert result == {"success": True} driver.create_radius_client("ap-lobby", "10.0.0.5/32", "s3cr3t")
assert calls[0] == ( assert calls[0] == (
"/api/freeradius/client/add_client", "/api/freeradius/client/add_client",
{"client": {"name": "ap-lobby", "ip": "10.0.0.5/32", "secret": "s3cr3t"}}, {"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 result["validations"] == {"client.name": "A value is required."}
assert calls == ["/api/freeradius/client/add_client"] 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: class TestDeleteRadiusClient:
def test_success_reconfigures_service(self, driver): def test_success_reconfigures_service(self, driver):
@@ -1246,11 +1258,17 @@ class TestGetRadiusUsers:
class TestCreateRadiusUser: 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 = [] calls = []
driver._post = lambda path, data=None: calls.append((path, data)) or {"result": "saved"} driver._post = lambda path, data=None: calls.append((path, data)) or {"result": "saved"}
result = driver.create_radius_user("jdoe", "hunter2") driver._get = lambda path: RADIUS_USERS_RESPONSE
assert result == {"success": True} driver.create_radius_user("jdoe", "hunter2")
assert calls[0] == ( assert calls[0] == (
"/api/freeradius/user/add_user", "/api/freeradius/user/add_user",
{"user": {"username": "jdoe", "password": "hunter2"}}, {"user": {"username": "jdoe", "password": "hunter2"}},
@@ -1267,6 +1285,12 @@ class TestCreateRadiusUser:
assert result["success"] is False assert result["success"] is False
assert calls == ["/api/freeradius/user/add_user"] 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: class TestDeleteRadiusUser:
def test_success_reconfigures_service(self, driver): def test_success_reconfigures_service(self, driver):