diff --git a/backend/app/configgen.py b/backend/app/configgen.py index b0fb4c6..7ab9fd7 100644 --- a/backend/app/configgen.py +++ b/backend/app/configgen.py @@ -38,10 +38,13 @@ def _listener_block(listener: ListenerConfig, warnings: list[str]) -> list[str]: lines: list[str] = [] if not listener.enabled: return lines - lines.append(f"listener {listener.port} {listener.bind_address or ''}".rstrip()) - if listener.bind_address is None: - # `listener ` without bind address binds all interfaces. - lines[-1] = f"listener {listener.port}" + # Always emit an EXPLICIT bind address. Mosquitto's reload compares + # listeners textually; "listener 1883" vs "listener 1883 0.0.0.0" are + # treated as different listeners, and reload CANNOT add/remove + # listeners (spec section 28). Matching the bootstrap's explicit + # form keeps the management listener reloadable. + bind = listener.bind_address or "0.0.0.0" + lines.append(f"listener {listener.port} {bind}") lines.append(f"protocol {listener.protocol}") if listener.allow_anonymous: lines.append("listener_allow_anonymous true") @@ -104,11 +107,17 @@ def generate_config(db: Session) -> GenerationResult: lines.extend(block) lines.append("") else: + # Empty model is a legitimate UI state (fresh platform): generate a + # SAFE config instead of raising, so /config/preview works. With no + # listener directives Mosquitto binds loopback-only (127.0.0.1/::1) + # on port 1883 with anonymous access — reachable only inside the + # container network. The warning explains exactly what will happen. warnings.append( - "no enabled listeners; broker would fall back to loopback-only " - "with anonymous access — refusing to generate" + "No enabled listeners in the configuration model. The generated " + "config contains no listener directives, so Mosquitto will bind " + "loopback-only (127.0.0.1/::1) on port 1883 with anonymous access " + "— internal to the container network only. Add a listener below." ) - raise ValueError("no enabled listeners in configuration model") lines.append("# --- Persistence ---") lines.append("persistence true") diff --git a/backend/app/routers/broker.py b/backend/app/routers/broker.py index fb4a331..aecb458 100644 --- a/backend/app/routers/broker.py +++ b/backend/app/routers/broker.py @@ -30,6 +30,7 @@ from app.models import ( ) from app.schemas import ( BrokerSettingsIn, + BrokerSettingsOut, ConfigPreviewOut, DeployRequest, ListenerIn, @@ -126,13 +127,21 @@ def update_listener( for key, value in body.model_dump().items(): setattr(listener, key, value) db.flush() + # Audit the security-relevant listener fields (spec section 38): the + # full PUT payload is a replacement, so record what it now says. audit.record( db, "LISTENER_UPDATED", request=request, object_type="listener", object_id=str(listener.id), - new_value={"name": body.name, "port": body.port}, + new_value={ + "name": body.name, + "port": body.port, + "bind_address": body.bind_address, + "protocol": body.protocol, + "allow_anonymous": body.allow_anonymous, + }, ) return listener @@ -161,7 +170,7 @@ def delete_listener( # --- Global settings ----------------------------------------------------------- -@router.get("/settings", response_model=list[BrokerSettingsIn]) +@router.get("/settings", response_model=list[BrokerSettingsOut]) def list_settings(db: Annotated[Session, Depends(get_db)], _user: AnyReader): return db.scalars(select(BrokerSettings)).all() diff --git a/backend/app/schemas.py b/backend/app/schemas.py index c4fa7d8..f7c56e8 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -185,6 +185,12 @@ class BrokerSettingsIn(BaseModel): value_type: Literal["string", "int", "bool"] = "string" +class BrokerSettingsOut(BrokerSettingsIn): + """GET /broker/settings rows — same fields, from ORM attributes.""" + + model_config = {"from_attributes": True} + + class ListenerIn(BaseModel): name: str = Field(min_length=1, max_length=100) enabled: bool = True diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 0ca93a2..766e16a 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -180,6 +180,37 @@ def test_listener_management_and_preview(admin_client): assert delete.status_code == 204 +def test_listener_update_and_preview(admin_client): + listener_id = admin_client.post( + "/api/v1/broker/listeners", + json={"name": "Production MQTT", "port": 1883, "protocol": "mqtt"}, + ).json()["id"] + + updated = admin_client.put( + f"/api/v1/broker/listeners/{listener_id}", + json={ + "name": "Production MQTT", + "port": 1884, + "bind_address": "127.0.0.1", + "protocol": "websockets", + "allow_anonymous": True, + }, + ) + assert updated.status_code == 200, updated.text + body = updated.json() + assert body["port"] == 1884 + assert body["bind_address"] == "127.0.0.1" + assert body["protocol"] == "websockets" + assert body["allow_anonymous"] is True + + config_text = admin_client.get("/api/v1/broker/config/preview").json()[ + "config_text" + ] + assert "listener 1884 127.0.0.1" in config_text + assert "protocol websockets" in config_text + assert "listener_allow_anonymous true" in config_text + + def test_audit_events_recorded(admin_client, db_session): from sqlalchemy import select diff --git a/backend/tests/test_configgen.py b/backend/tests/test_configgen.py index 090d57e..b3a84ae 100644 --- a/backend/tests/test_configgen.py +++ b/backend/tests/test_configgen.py @@ -46,9 +46,12 @@ def test_generate_includes_global_settings(session): assert "max_packet_size 2000000" in result.config_text -def test_generate_refuses_without_listeners(session): - with pytest.raises(ValueError, match="no enabled listeners"): - generate_config(session) +def test_generate_with_empty_model_warns_instead_of_raising(session): + """Fresh-platform state: preview must work, not 500 (config page fix).""" + result = generate_config(session) + assert "global_plugin /usr/lib/mosquitto_dynamic_security.so" in result.config_text + assert "persistence true" in result.config_text + assert any("No enabled listeners" in w for w in result.warnings) def test_disabled_listeners_are_omitted(session): diff --git a/broker-control/control.py b/broker-control/control.py index 86dd6b9..49c8994 100644 --- a/broker-control/control.py +++ b/broker-control/control.py @@ -130,13 +130,18 @@ def _post_broker_health() -> bool: Uses mosquitto_ctrl rather than a $SYS subscription, because the 2.1 auto-generated admin role set is not guaranteed to include $SYS read access — but it must always be able to administer dynamic security. + + NOTE: this sidecar shares the broker's PID namespace but NOT its + network namespace — 127.0.0.1 here is the SIDECAR's loopback, where + nothing listens. The broker is reached via Docker DNS ("mosquitto") + on the backend-net network. """ username = os.environ.get("MOSQUITTO_DYNSEC_USERNAME", "admin") token_pw = os.environ.get("MOSQUITTO_DYNSEC_PASSWORD", "") cmd = [ "mosquitto_ctrl", "-h", - "127.0.0.1", + "mosquitto", "-u", username, "-P", diff --git a/compose.yaml b/compose.yaml index ba16956..f6124b7 100644 --- a/compose.yaml +++ b/compose.yaml @@ -14,6 +14,7 @@ x-backend-env: &backend-env MOSQUITTO_MGMT_PORT: ${MOSQUITTO_MGMT_PORT:-1883} MOSQUITTO_DYNSEC_USERNAME: ${MOSQUITTO_DYNSEC_USERNAME:-admin} MOSQUITTO_DYNSEC_PASSWORD: ${MOSQUITTO_DYNSEC_PASSWORD:?MOSQUITTO_DYNSEC_PASSWORD must be set} + CONTROL_API_TOKEN: ${CONTROL_API_TOKEN:?CONTROL_API_TOKEN must be set} DEFAULT_TLS_MIN_VERSION: ${DEFAULT_TLS_MIN_VERSION:-tlsv1.2} DEFAULT_CERT_VALIDITY_DAYS: ${DEFAULT_CERT_VALIDITY_DAYS:-365} DEFAULT_CA_VALIDITY_DAYS: ${DEFAULT_CA_VALIDITY_DAYS:-3650} diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 5bbeb05..0bf7a02 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,4 +1,4 @@ -import { NavLink, Navigate, Route, Routes } from "react-router-dom"; +import { NavLink, Navigate, Route, Routes, useLocation } from "react-router-dom"; import { useAuth } from "./auth"; import { LoginPage } from "./pages/Login"; import { ClientsPage } from "./pages/Clients"; @@ -30,8 +30,12 @@ function Layout({ children }: { children: React.ReactNode }) { function Protected({ children }: { children: React.ReactNode }) { const { session, loading } = useAuth(); + const location = useLocation(); if (loading) return

Loading…

; - if (!session) return ; + if (!session) { + // Remember the guarded route so Login can return here after sign-in. + return ; + } return {children}; } diff --git a/frontend/src/api.ts b/frontend/src/api.ts index 81aa408..cc2d722 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -41,6 +41,24 @@ function errorMessage(body: unknown): string | null { return null; } +// A 401 here means the session cookie expired or was never valid. +// api.ts lives outside the React router, so the only way to the login +// screen is a full-page navigation; the current page is remembered via +// ?next= so Login can return after sign-in. +// Excluded: the login attempt itself (a 401 there is bad credentials and +// must render the error banner, not navigate away), the startup session +// probe (a 401 there is the normal "not signed in" state, which +// turns into an in-app redirect instead), and logout (a 401 +// there just means the session already expired — auth.tsx clears the +// client session anyway, so no redirect is needed). +const EXPECTED_401_PATHS = new Set(["/auth/login", "/auth/session", "/auth/logout"]); + +function redirectToLogin(): void { + const current = window.location.pathname; + if (current === "/login") return; // defensive: never reload the login page + window.location.replace(`/login?next=${encodeURIComponent(current)}`); +} + async function request(path: string, init?: RequestInit): Promise { const response = await fetch(`${BASE}${path}`, { credentials: "include", @@ -55,6 +73,9 @@ async function request(path: string, init?: RequestInit): Promise { } catch { /* non-JSON error body */ } + if (response.status === 401 && !EXPECTED_401_PATHS.has(path)) { + redirectToLogin(); + } throw new ApiError(response.status, message); } if (response.status === 204) return undefined as T; diff --git a/frontend/src/auth.tsx b/frontend/src/auth.tsx index cdd07b2..54685ec 100644 --- a/frontend/src/auth.tsx +++ b/frontend/src/auth.tsx @@ -34,7 +34,10 @@ export function AuthProvider({ children }: { children: ReactNode }) { }, []); const logout = useCallback(async () => { - await api.post("/auth/logout"); + // A 401 here means the session already expired server-side; clearing + // the client session is still the right outcome (api.ts does not + // redirect for /auth/logout), and then shows /login. + await api.post("/auth/logout").catch(() => undefined); setSession(null); }, []); diff --git a/frontend/src/pages/BrokerConfig.tsx b/frontend/src/pages/BrokerConfig.tsx index 94032a9..b3d7bdd 100644 --- a/frontend/src/pages/BrokerConfig.tsx +++ b/frontend/src/pages/BrokerConfig.tsx @@ -1,4 +1,5 @@ -import { useMutation, useQuery } from "@tanstack/react-query"; +import { FormEvent, useEffect, useState } from "react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { api, type Listener, type Revision } from "../api"; interface ConfigPreview { @@ -6,22 +7,80 @@ interface ConfigPreview { warnings: string[]; } +interface BrokerSetting { + key: string; + value: string; + value_type: "string" | "int" | "bool"; +} + +const EMPTY_LISTENER: Omit = { + name: "", + enabled: true, + port: 1883, + bind_address: null, + protocol: "mqtt", + allow_anonymous: false, + max_connections: -1, + tls_enabled: false, + tls_min_version: null, + require_certificate: false, + use_identity_as_username: false, +}; + +// Curated subset of spec section 31; anything else can be added later +// through the same settings endpoint (advanced editor is a later release). +const COMMON_SETTINGS: { key: string; label: string; placeholder: string; hint: string }[] = [ + { key: "max_packet_size", label: "max_packet_size", placeholder: "2000000", hint: "bytes; 2.1 default is 2 MB" }, + { key: "autosave_interval", label: "autosave_interval", placeholder: "900", hint: "seconds between persistence saves" }, + { key: "sys_interval", label: "sys_interval", placeholder: "10", hint: "$SYS update interval (s)" }, + { key: "message_size_limit", label: "message_size_limit", placeholder: "0", hint: "max payload bytes; 0 = unlimited" }, + { key: "max_queued_messages", label: "max_queued_messages", placeholder: "1000", hint: "per-client queue for offline clients" }, + { key: "global_max_clients", label: "global_max_clients", placeholder: "-1", hint: "total sessions; -1 = unlimited" }, +]; + export function BrokerConfigPage() { + const queryClient = useQueryClient(); + const [editingId, setEditingId] = useState(null); + const refresh = () => { + queryClient.invalidateQueries({ queryKey: ["listeners"] }); + queryClient.invalidateQueries({ queryKey: ["settings"] }); + queryClient.invalidateQueries({ queryKey: ["config-preview"] }); + }; + const listeners = useQuery({ queryKey: ["listeners"], queryFn: () => api.get("/broker/listeners"), }); - + const settings = useQuery({ + queryKey: ["settings"], + queryFn: () => api.get("/broker/settings"), + }); const preview = useQuery({ queryKey: ["config-preview"], queryFn: () => api.get("/broker/config/preview"), }); - const revisions = useQuery({ queryKey: ["revisions"], queryFn: () => api.get("/broker/revisions"), }); + const addListener = useMutation({ + mutationFn: (l: Omit) => api.post("/broker/listeners", l), + onSuccess: refresh, + }); + const updateListener = useMutation({ + mutationFn: ({ id, ...l }: Listener) => api.put(`/broker/listeners/${id}`, l), + onSuccess: refresh, + }); + const removeListener = useMutation({ + mutationFn: (id: string) => api.delete(`/broker/listeners/${id}`), + onSuccess: refresh, + }); + const putSetting = useMutation({ + mutationFn: ({ key, value }: { key: string; value: string }) => + api.put(`/broker/settings/${key}`, { key, value, value_type: "string" }), + onSuccess: refresh, + }); const deploy = useMutation({ mutationFn: (comment: string | null) => api.post<{ status: string; revision: number; broker_healthy: boolean }>( @@ -29,8 +88,8 @@ export function BrokerConfigPage() { { comment }, ), onSuccess: () => { - preview.refetch(); - revisions.refetch(); + refresh(); + queryClient.invalidateQueries({ queryKey: ["revisions"] }); }, }); @@ -38,28 +97,52 @@ export function BrokerConfigPage() {

Broker configuration

- The configuration is generated from the platform model, validated with - mosquitto --test-config, deployed atomically, and every + The configuration is generated from the model below, validated with{" "} + mosquitto --test-config, deployed atomically, and every deployment creates a revision.

+ {(listeners.error || preview.error || settings.error) && ( +
+ {((listeners.error ?? preview.error ?? settings.error) as Error).message} +
+ )} -

Listeners

- {listeners.isLoading ? ( -

Loading…

- ) : ( - - - - - - - - - - - - - {(listeners.data ?? []).map((l) => ( +

Add listener

+ addListener.mutate(l)} pending={addListener.isPending} /> + {addListener.error && ( +
{(addListener.error as Error).message}
+ )} + +

Listeners

+
NamePortBindProtocolAnonymousEnabled
+ + + + + + + + + + + + + {(listeners.data ?? []).map((l) => + editingId === l.id ? ( + + + + ) : ( @@ -67,12 +150,59 @@ export function BrokerConfigPage() { + - ))} - -
NamePortBindProtocolAnonymousEnabled
+ setEditingId(null)} + onSubmit={(patch) => { + updateListener.mutate({ ...l, ...patch }); + setEditingId(null); + }} + /> +
{l.name} {l.port}{l.protocol} {l.allow_anonymous ? "yes" : "no"} {l.enabled ? "yes" : "no"} + {" "} + {" "} + +
+ ), + )} + + + {(listeners.data ?? []).length === 0 && !listeners.isLoading && ( +

+ No listeners yet. Add one above — after a deploy, the broker is only + reachable through the listeners defined here. +

)} +

Global settings

+ + + + + + + + + + {COMMON_SETTINGS.map((s) => { + const existing = (settings.data ?? []).find((row) => row.key === s.key); + return ( + putSetting.mutate({ key: s.key, value })} + /> + ); + })} + +
SettingValue
+

Generated configuration

{preview.data?.warnings.map((w) => (
@@ -126,4 +256,148 @@ export function BrokerConfigPage() {
); +} + +function ListenerForm({ + initial, + onSubmit, + onCancel, + pending, +}: { + initial?: Listener; + onSubmit: (l: Omit) => void; + onCancel?: () => void; + pending: boolean; +}) { + const [draft, setDraft] = useState>(initial ?? EMPTY_LISTENER); + useEffect(() => { + setDraft(initial ?? EMPTY_LISTENER); + }, [initial]); + const set = (patch: Partial) => setDraft((d) => ({ ...d, ...patch })); + const valid = draft.name.trim() !== "" && draft.port > 0 && draft.port <= 65535; + + const submit = (e: FormEvent) => { + e.preventDefault(); + if (valid) { + onSubmit(draft); + if (!initial) setDraft(EMPTY_LISTENER); + } + }; + + return ( +
+ {initial && ( +
+ Port or bind changes require a broker restart — a config reload + cannot modify existing listener definitions. +
+ )} +
+
+ + set({ name: e.target.value })} + placeholder="Production MQTT" + /> +
+
+ + set({ port: Number(e.target.value) })} + /> +
+
+ + +
+
+ + set({ bind_address: e.target.value || null })} + placeholder="all interfaces" + /> +
+
+ + +
+
+ + set({ max_connections: Number(e.target.value) })} + /> +
+
+

+ {" "} + {initial && onCancel && ( + + )} +

+ ); +} + +function SettingsRow({ + label, + hint, + placeholder, + initial, + pending, + onSave, +}: { + label: string; + hint: string; + placeholder: string; + initial?: string; + pending: boolean; + onSave: (value: string) => void; +}) { + const [value, setValue] = useState(initial ?? ""); + useEffect(() => { + setValue(initial ?? ""); + }, [initial]); + + return ( + + + {label} +
{hint}
+ + + setValue(e.target.value)} + style={{ maxWidth: "220px" }} + /> + + + + + + ); } \ No newline at end of file diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx index 0496eb8..07581a6 100644 --- a/frontend/src/pages/Login.tsx +++ b/frontend/src/pages/Login.tsx @@ -1,19 +1,41 @@ import { FormEvent, useState } from "react"; +import { useLocation, useNavigate, useSearchParams } from "react-router-dom"; import { useAuth } from "../auth"; +// Only same-app targets are allowed: blocks open redirects such as +// "//evil.host" or "https://evil.host" via the ?next= query parameter. +function safeTarget(candidate: string | null): string { + return candidate && candidate.startsWith("/") && !candidate.startsWith("//") + ? candidate + : "/clients"; +} + export function LoginPage() { const { login } = useAuth(); + const navigate = useNavigate(); + const location = useLocation(); + const [searchParams] = useSearchParams(); const [username, setUsername] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState(null); const [busy, setBusy] = useState(false); + // Where to go after sign-in. passes the guarded route via + // router state (in-app navigation); the 401 handler in api.ts passes the + // current page as ?next= (full-page navigation). Direct /login visits + // default to /clients. + const from = safeTarget( + (location.state as { from?: string } | null)?.from ?? searchParams.get("next"), + ); + const submit = async (e: FormEvent) => { e.preventDefault(); setBusy(true); setError(null); try { await login(username, password); + // replace: Back must not return to the login screen after sign-in. + navigate(from, { replace: true }); } catch { setError("Invalid username or password."); } finally { diff --git a/mosquitto/bootstrap/mosquitto.conf b/mosquitto/bootstrap/mosquitto.conf index 2ceaa7b..a9e22d8 100644 --- a/mosquitto/bootstrap/mosquitto.conf +++ b/mosquitto/bootstrap/mosquitto.conf @@ -17,6 +17,10 @@ # Plaintext is acceptable here because the network is isolated and the # listener is not published to the host. Production TLS listeners are # added via the platform (spec §60). +# NOTE: the platform generator always emits explicit bind addresses for +# listeners — Mosquitto's reload compares listeners textually and refuses +# to add/remove listeners on reload (spec §28). This line matches that +# form so the first platform deploy is a pure reload, not a restart. listener 1883 0.0.0.0 allow_anonymous false