From adbaab50b316d2dca9012b1a3825ff1937e8c4b4 Mon Sep 17 00:00:00 2001 From: Jonas Bengtsson Date: Sat, 19 Sep 2026 12:00:34 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A7=20WIP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/app/routers/mqtt.py | 40 ++++++++++++++++++++++++++++++++++++- backend/tests/test_api.py | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/backend/app/routers/mqtt.py b/backend/app/routers/mqtt.py index c4cc6ee..53ec83e 100644 --- a/backend/app/routers/mqtt.py +++ b/backend/app/routers/mqtt.py @@ -13,6 +13,7 @@ from fastapi import APIRouter, Depends, HTTPException, Request, status from sqlalchemy.orm import Session from app import audit +from app.config import get_settings from app.db import get_db from app.deps import CurrentUser, require_roles from app.dynsec import DynsecError, dynsec_client @@ -36,6 +37,25 @@ BrokerAdmin = Annotated[CurrentUser, Depends(require_roles(ManagementRole.BROKER DbDep = Annotated[Session, Depends(get_db)] +def _guard_mgmt_identity(username: str) -> None: + """Refuse operations on the backend's own management identity. + + The dynsec client named MOSQUITTO_DYNSEC_USERNAME is the platform's + control-channel service account — the backend connects to the broker + AS this identity (spec section 2.1: management identities are separate + from MQTT identities). Resetting its password or disabling it severs + the management channel: Mosquitto kicks every connection using the + username, and the backend then cannot reconnect with its env-var + password. It is therefore not a manageable MQTT client. + """ + if username == get_settings().mosquitto_dynsec_username: + raise HTTPException( + status.HTTP_400_BAD_REQUEST, + f"'{username}' is the broker management identity and cannot " + "be modified through client management", + ) + + def _metadata(db: Session, username: str) -> MqttClientMetadata | None: return db.get(MqttClientMetadata, username) @@ -63,7 +83,12 @@ def _client_out(db: Session, client: dict) -> MqttClientOut: @router.get("/clients", response_model=list[MqttClientOut]) def list_clients(db: DbDep, _user: BrokerAdmin): - return [_client_out(db, c) for c in dynsec_client.list_clients()] + mgmt = get_settings().mosquitto_dynsec_username + return [ + _client_out(db, c) + for c in dynsec_client.list_clients() + if c.get("username") != mgmt + ] @router.post("/clients", response_model=MqttPasswordReset, status_code=201) @@ -74,6 +99,7 @@ def create_client( _user: BrokerAdmin, ): """Create a dynsec client; the plaintext password is returned ONCE.""" + _guard_mgmt_identity(body.username) password = body.password or generate_password() try: dynsec_client.create_client( @@ -109,6 +135,10 @@ def create_client( @router.get("/clients/{username}", response_model=MqttClientOut) def get_client(username: str, db: DbDep, _user: BrokerAdmin): + # Hidden like in the listing: the management identity is not a + # manageable client, so it is reported as not present. + if username == get_settings().mosquitto_dynsec_username: + raise HTTPException(status.HTTP_404_NOT_FOUND, "client not found") try: client = dynsec_client.get_client(username) except DynsecError as exc: @@ -123,6 +153,7 @@ def delete_client( db: DbDep, _user: BrokerAdmin, ): + _guard_mgmt_identity(username) try: dynsec_client.delete_client(username) except DynsecError as exc: @@ -144,6 +175,7 @@ def delete_client( def disable_client( username: str, request: Request, db: DbDep, _user: BrokerAdmin ): + _guard_mgmt_identity(username) dynsec_client.disable_client(username) audit.record( db, "CLIENT_DISABLED", request=request, object_type="mqtt_client", object_id=username @@ -155,6 +187,7 @@ def disable_client( def enable_client( username: str, request: Request, db: DbDep, _user: BrokerAdmin ): + _guard_mgmt_identity(username) dynsec_client.enable_client(username) audit.record( db, "CLIENT_ENABLED", request=request, object_type="mqtt_client", object_id=username @@ -170,6 +203,7 @@ def reset_password( _user: BrokerAdmin, ): """Generate a new random password; returned once (spec section 11).""" + _guard_mgmt_identity(username) password = generate_password() try: dynsec_client.set_client_password(username, password) @@ -194,6 +228,7 @@ def assign_client_role( _user: BrokerAdmin, ): """Add one role to an existing client (incremental, spec section 11).""" + _guard_mgmt_identity(username) try: dynsec_client.add_client_role(username, body.rolename, body.priority) except DynsecError as exc: @@ -219,6 +254,7 @@ def unassign_client_role( db: DbDep, _user: BrokerAdmin, ): + _guard_mgmt_identity(username) try: dynsec_client.remove_client_role(username, rolename) except DynsecError as exc: @@ -290,6 +326,7 @@ def add_group_member( _user: BrokerAdmin, ): """Add one client to a group (spec section 12).""" + _guard_mgmt_identity(body.username) try: dynsec_client.add_group_client(groupname, body.username, body.priority) except DynsecError as exc: @@ -315,6 +352,7 @@ def remove_group_member( db: DbDep, _user: BrokerAdmin, ): + _guard_mgmt_identity(username) try: dynsec_client.remove_group_client(groupname, username) except DynsecError as exc: diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 766e16a..c478894 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -63,6 +63,45 @@ def test_disable_enable_delete_client(admin_client): assert response.status_code == 204 +def test_management_identity_is_protected(admin_client): + """The dynsec admin (MOSQUITTO_DYNSEC_USERNAME) is the backend's own + service account: hidden from the listing and immune to password reset, + disable and delete. A reset previously severed the management channel + (Mosquitto kicks all connections for the username) — hence the guard. + """ + # Create "admin" directly so the filter/guard has something to catch. + # (The FakeDynsec fixture is empty at test start.) + admin_client.post("/api/v1/mqtt/clients", json={"username": "admin"}) + + # Hidden from the listing even though dynsec knows the client. + listing = admin_client.get("/api/v1/mqtt/clients").json() + assert all(c["username"] != "admin" for c in listing) + assert admin_client.get("/api/v1/mqtt/clients/admin").status_code == 404 + + # Every mutation targeting it returns 400, not 2xx. + assert ( + admin_client.post("/api/v1/mqtt/clients/admin/password:reset").status_code + == 400 + ) + assert admin_client.post("/api/v1/mqtt/clients/admin/disable").status_code == 400 + assert admin_client.delete("/api/v1/mqtt/clients/admin").status_code == 400 + # Creating a client that shadows the identity is refused as well. + assert ( + admin_client.post("/api/v1/mqtt/clients", json={"username": "admin"}).status_code + == 400 + ) + + +def test_password_reset_still_works_for_normal_clients(admin_client): + admin_client.post("/api/v1/mqtt/clients", json={"username": "ps042"}) + + reset = admin_client.post("/api/v1/mqtt/clients/ps042/password:reset") + assert reset.status_code == 200 + body = reset.json() + assert body["username"] == "ps042" + assert len(body["password"]) >= 20 # random password, shown once + + def test_role_acl_lifecycle(admin_client): admin_client.post("/api/v1/mqtt/roles", json={"rolename": "telemetry"})