feat: switch federation proxy endpoints to device_id lookup (task-9)
Change all 5 federation proxy URL patterns from {remote_id:int} to
{device_id} (str) and use _lookup_remote_by_device_id() for lookup.
Affected endpoints:
- federation_connect: /api/federation/{device_id}/connect/{session_name}
- federation_bell_clear: /api/federation/{device_id}/sessions/{session_name}/bell/clear
- federation_create_session: /api/federation/{device_id}/sessions
- federation_delete_session: /api/federation/{device_id}/sessions/{session_name}
- federation_terminal_ws_proxy: /federation/{device_id}/terminal/ws
All endpoints now use _lookup_remote_by_device_id(device_id) which
provides integer index fallback for backward compatibility.
Also update existing test to expect 404 instead of 422 for non-integer
device_id (since str type accepts any string, lookup returns None->404).
Tests:
- test_federation_connect_by_device_id: POST with device_id='aaa-111-bbb' returns 200
- test_federation_connect_device_id_not_found: unknown device_id returns 404
This commit is contained in:
+37
-52
@@ -242,9 +242,7 @@ async def _run_poll_cycle() -> None:
|
|||||||
try:
|
try:
|
||||||
await _federation_client.post(
|
await _federation_client.post(
|
||||||
bell_clear_url,
|
bell_clear_url,
|
||||||
headers={"Authorization": f"Bearer {remote_key}"}
|
headers={"Authorization": f"Bearer {remote_key}"} if remote_key else {},
|
||||||
if remote_key
|
|
||||||
else {},
|
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
_log.warning(
|
_log.warning(
|
||||||
@@ -1047,29 +1045,26 @@ def _lookup_remote_by_device_id(device_id: str) -> dict | None:
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@app.websocket("/federation/{remote_id}/terminal/ws")
|
@app.websocket("/federation/{device_id}/terminal/ws")
|
||||||
async def federation_terminal_ws_proxy(websocket: WebSocket, remote_id: int) -> None:
|
async def federation_terminal_ws_proxy(websocket: WebSocket, device_id: str) -> None:
|
||||||
"""Proxy WebSocket frames between the browser and a remote muxplex ttyd.
|
"""Proxy WebSocket frames between the browser and a remote muxplex ttyd.
|
||||||
|
|
||||||
*remote_id* is the integer index into the ``remote_instances`` list in
|
*device_id* is the device_id string of the remote instance in
|
||||||
settings. Authenticates to the remote instance using the configured
|
settings. Authenticates to the remote instance using the configured
|
||||||
``key`` field via a Bearer header.
|
``key`` field via a Bearer header.
|
||||||
|
|
||||||
Auth check uses the same cookie + bearer pattern as terminal_ws_proxy.
|
Auth check uses the same cookie + bearer pattern as terminal_ws_proxy.
|
||||||
Closes with code 4004 if remote_id is out of range.
|
Closes with code 4004 if device_id does not match any remote.
|
||||||
"""
|
"""
|
||||||
# Auth check before accepting — same pattern as terminal_ws_proxy
|
# Auth check before accepting — same pattern as terminal_ws_proxy
|
||||||
if not await _ws_auth_check(websocket):
|
if not await _ws_auth_check(websocket):
|
||||||
return
|
return
|
||||||
|
|
||||||
# Look up remote instance by index
|
# Look up remote instance by device_id
|
||||||
settings = load_settings()
|
remote = _lookup_remote_by_device_id(device_id)
|
||||||
remote_instances: list[dict] = settings.get("remote_instances", [])
|
if remote is None:
|
||||||
if remote_id < 0 or remote_id >= len(remote_instances):
|
|
||||||
await websocket.close(code=4004)
|
await websocket.close(code=4004)
|
||||||
return
|
return
|
||||||
|
|
||||||
remote = remote_instances[remote_id]
|
|
||||||
remote_url: str = remote.get("url", "").rstrip("/")
|
remote_url: str = remote.get("url", "").rstrip("/")
|
||||||
remote_key: str = remote.get("key", "")
|
remote_key: str = remote.get("key", "")
|
||||||
|
|
||||||
@@ -1098,9 +1093,7 @@ async def federation_terminal_ws_proxy(websocket: WebSocket, remote_id: int) ->
|
|||||||
async with websockets.connect(
|
async with websockets.connect(
|
||||||
ws_url,
|
ws_url,
|
||||||
subprotocols=[Subprotocol("tty")],
|
subprotocols=[Subprotocol("tty")],
|
||||||
additional_headers={"Authorization": f"Bearer {remote_key}"}
|
additional_headers={"Authorization": f"Bearer {remote_key}"} if remote_key else {},
|
||||||
if remote_key
|
|
||||||
else {},
|
|
||||||
ssl=ssl_context,
|
ssl=ssl_context,
|
||||||
) as remote_ws:
|
) as remote_ws:
|
||||||
|
|
||||||
@@ -1373,26 +1366,24 @@ async def federation_generate_key() -> dict:
|
|||||||
return {"key": key, "path": str(path)}
|
return {"key": key, "path": str(path)}
|
||||||
|
|
||||||
|
|
||||||
@app.post("/api/federation/{remote_id}/connect/{session_name}")
|
@app.post("/api/federation/{device_id}/connect/{session_name}")
|
||||||
async def federation_connect(
|
async def federation_connect(
|
||||||
remote_id: int, session_name: str, request: Request
|
device_id: str, session_name: str, request: Request
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Proxy a connect POST to a remote instance to spawn its ttyd.
|
"""Proxy a connect POST to a remote instance to spawn its ttyd.
|
||||||
|
|
||||||
Looks up the remote by integer index into ``remote_instances`` in settings,
|
Looks up the remote by device_id string via ``_lookup_remote_by_device_id``,
|
||||||
sends ``POST {remote_url}/api/sessions/{session_name}/connect`` with a
|
sends ``POST {remote_url}/api/sessions/{session_name}/connect`` with a
|
||||||
Bearer auth header, and returns the remote's JSON response.
|
Bearer auth header, and returns the remote's JSON response.
|
||||||
|
|
||||||
Raises HTTP 404 if ``remote_id`` is not a valid integer index.
|
Raises HTTP 404 if ``device_id`` does not match any remote instance.
|
||||||
"""
|
"""
|
||||||
settings = load_settings()
|
remote = _lookup_remote_by_device_id(device_id)
|
||||||
remotes = settings.get("remote_instances", [])
|
if remote is None:
|
||||||
if remote_id < 0 or remote_id >= len(remotes):
|
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=404,
|
status_code=404,
|
||||||
detail=f"Remote instance '{remote_id}' not found",
|
detail=f"Remote instance '{device_id}' not found",
|
||||||
)
|
)
|
||||||
remote = remotes[remote_id]
|
|
||||||
|
|
||||||
remote_url: str = remote.get("url", "").rstrip("/")
|
remote_url: str = remote.get("url", "").rstrip("/")
|
||||||
remote_key: str = remote.get("key", "")
|
remote_key: str = remote.get("key", "")
|
||||||
@@ -1419,26 +1410,24 @@ async def federation_connect(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@app.post("/api/federation/{remote_id}/sessions/{session_name}/bell/clear")
|
@app.post("/api/federation/{device_id}/sessions/{session_name}/bell/clear")
|
||||||
async def federation_bell_clear(
|
async def federation_bell_clear(
|
||||||
remote_id: int, session_name: str, request: Request
|
device_id: str, session_name: str, request: Request
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Proxy a bell-clear POST to a remote instance.
|
"""Proxy a bell-clear POST to a remote instance.
|
||||||
|
|
||||||
Looks up the remote by integer index into ``remote_instances`` in settings,
|
Looks up the remote by device_id string via ``_lookup_remote_by_device_id``,
|
||||||
sends ``POST {remote_url}/api/sessions/{session_name}/bell/clear`` with a
|
sends ``POST {remote_url}/api/sessions/{session_name}/bell/clear`` with a
|
||||||
Bearer auth header, and returns the remote's JSON response.
|
Bearer auth header, and returns the remote's JSON response.
|
||||||
|
|
||||||
Raises HTTP 404 if ``remote_id`` is not a valid integer index.
|
Raises HTTP 404 if ``device_id`` does not match any remote instance.
|
||||||
"""
|
"""
|
||||||
settings = load_settings()
|
remote = _lookup_remote_by_device_id(device_id)
|
||||||
remotes = settings.get("remote_instances", [])
|
if remote is None:
|
||||||
if remote_id < 0 or remote_id >= len(remotes):
|
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=404,
|
status_code=404,
|
||||||
detail=f"Remote instance '{remote_id}' not found",
|
detail=f"Remote instance '{device_id}' not found",
|
||||||
)
|
)
|
||||||
remote = remotes[remote_id]
|
|
||||||
|
|
||||||
remote_url: str = remote.get("url", "").rstrip("/")
|
remote_url: str = remote.get("url", "").rstrip("/")
|
||||||
remote_key: str = remote.get("key", "")
|
remote_key: str = remote.get("key", "")
|
||||||
@@ -1467,27 +1456,25 @@ async def federation_bell_clear(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@app.post("/api/federation/{remote_id}/sessions")
|
@app.post("/api/federation/{device_id}/sessions")
|
||||||
async def federation_create_session(
|
async def federation_create_session(
|
||||||
remote_id: int, payload: CreateSessionPayload, request: Request
|
device_id: str, payload: CreateSessionPayload, request: Request
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Proxy a create-session POST to a remote instance.
|
"""Proxy a create-session POST to a remote instance.
|
||||||
|
|
||||||
Looks up the remote by integer index into ``remote_instances`` in settings,
|
Looks up the remote by device_id string via ``_lookup_remote_by_device_id``,
|
||||||
sends ``POST {remote_url}/api/sessions`` with a Bearer auth header and JSON
|
sends ``POST {remote_url}/api/sessions`` with a Bearer auth header and JSON
|
||||||
body ``{name: ...}``, and returns the remote's JSON response.
|
body ``{name: ...}``, and returns the remote's JSON response.
|
||||||
|
|
||||||
Raises HTTP 404 if ``remote_id`` is not a valid integer index,
|
Raises HTTP 404 if ``device_id`` does not match any remote instance,
|
||||||
503 when remote is unreachable, 502 when remote returns HTTP error.
|
503 when remote is unreachable, 502 when remote returns HTTP error.
|
||||||
"""
|
"""
|
||||||
settings = load_settings()
|
remote = _lookup_remote_by_device_id(device_id)
|
||||||
remotes = settings.get("remote_instances", [])
|
if remote is None:
|
||||||
if remote_id < 0 or remote_id >= len(remotes):
|
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=404,
|
status_code=404,
|
||||||
detail=f"Remote instance '{remote_id}' not found",
|
detail=f"Remote instance '{device_id}' not found",
|
||||||
)
|
)
|
||||||
remote = remotes[remote_id]
|
|
||||||
remote_url: str = remote.get("url", "").rstrip("/")
|
remote_url: str = remote.get("url", "").rstrip("/")
|
||||||
remote_key: str = remote.get("key", "")
|
remote_key: str = remote.get("key", "")
|
||||||
url = f"{remote_url}/api/sessions"
|
url = f"{remote_url}/api/sessions"
|
||||||
@@ -1515,27 +1502,25 @@ async def federation_create_session(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@app.delete("/api/federation/{remote_id}/sessions/{session_name}")
|
@app.delete("/api/federation/{device_id}/sessions/{session_name}")
|
||||||
async def federation_delete_session(
|
async def federation_delete_session(
|
||||||
remote_id: int, session_name: str, request: Request
|
device_id: str, session_name: str, request: Request
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Proxy a delete-session DELETE to a remote instance.
|
"""Proxy a delete-session DELETE to a remote instance.
|
||||||
|
|
||||||
Looks up the remote by integer index into ``remote_instances`` in settings,
|
Looks up the remote by device_id string via ``_lookup_remote_by_device_id``,
|
||||||
sends ``DELETE {remote_url}/api/sessions/{session_name}`` with a Bearer auth
|
sends ``DELETE {remote_url}/api/sessions/{session_name}`` with a Bearer auth
|
||||||
header, and returns the remote's JSON response.
|
header, and returns the remote's JSON response.
|
||||||
|
|
||||||
Raises HTTP 404 if ``remote_id`` is not a valid integer index,
|
Raises HTTP 404 if ``device_id`` does not match any remote instance,
|
||||||
503 when remote is unreachable, 502 when remote returns HTTP error.
|
503 when remote is unreachable, 502 when remote returns HTTP error.
|
||||||
"""
|
"""
|
||||||
settings = load_settings()
|
remote = _lookup_remote_by_device_id(device_id)
|
||||||
remotes = settings.get("remote_instances", [])
|
if remote is None:
|
||||||
if remote_id < 0 or remote_id >= len(remotes):
|
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=404,
|
status_code=404,
|
||||||
detail=f"Remote instance '{remote_id}' not found",
|
detail=f"Remote instance '{device_id}' not found",
|
||||||
)
|
)
|
||||||
remote = remotes[remote_id]
|
|
||||||
remote_url: str = remote.get("url", "").rstrip("/")
|
remote_url: str = remote.get("url", "").rstrip("/")
|
||||||
remote_key: str = remote.get("key", "")
|
remote_key: str = remote.get("key", "")
|
||||||
url = f"{remote_url}/api/sessions/{session_name}"
|
url = f"{remote_url}/api/sessions/{session_name}"
|
||||||
|
|||||||
@@ -2127,10 +2127,11 @@ def test_federation_connect_returns_404_for_invalid_remote_id(
|
|||||||
def test_federation_connect_returns_404_for_non_integer_remote_id(
|
def test_federation_connect_returns_404_for_non_integer_remote_id(
|
||||||
client, monkeypatch, tmp_path
|
client, monkeypatch, tmp_path
|
||||||
):
|
):
|
||||||
"""POST /api/federation/{remote_id}/connect/{session_name} returns 422 when remote_id is not an integer.
|
"""POST /api/federation/{device_id}/connect/{session_name} returns 404 when device_id has no match.
|
||||||
|
|
||||||
With remote_id typed as int, FastAPI validates the path parameter at the
|
With device_id typed as str, any string is a valid path parameter.
|
||||||
framework level and returns 422 Unprocessable Entity for non-integer values.
|
When no remote matches 'not-an-int' by device_id and it cannot be parsed
|
||||||
|
as an integer index, the lookup returns None and the endpoint returns 404.
|
||||||
"""
|
"""
|
||||||
import json
|
import json
|
||||||
|
|
||||||
@@ -2141,7 +2142,7 @@ def test_federation_connect_returns_404_for_non_integer_remote_id(
|
|||||||
settings_path.write_text(json.dumps({"remote_instances": []}))
|
settings_path.write_text(json.dumps({"remote_instances": []}))
|
||||||
|
|
||||||
response = client.post("/api/federation/not-an-int/connect/my-session")
|
response = client.post("/api/federation/not-an-int/connect/my-session")
|
||||||
assert response.status_code == 422 # FastAPI rejects non-integer path param
|
assert response.status_code == 404 # device_id not found in remote_instances
|
||||||
|
|
||||||
|
|
||||||
def test_federation_connect_returns_503_when_remote_unreachable(
|
def test_federation_connect_returns_503_when_remote_unreachable(
|
||||||
@@ -3365,3 +3366,85 @@ def test_lookup_remote_by_device_id_not_found(tmp_path, monkeypatch):
|
|||||||
assert result is None, (
|
assert result is None, (
|
||||||
f"Expected None for unknown device_id 'zzz-999', got: {result!r}"
|
f"Expected None for unknown device_id 'zzz-999', got: {result!r}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Task-9: Federation Proxy Endpoints — Switch to device_id Lookup
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_federation_connect_by_device_id(client, monkeypatch, tmp_path):
|
||||||
|
"""POST /api/federation/{device_id}/connect/{session_name} works when device_id matches a remote.
|
||||||
|
|
||||||
|
Configures a remote with device_id='aaa-111-bbb', POSTs to the new device_id-based
|
||||||
|
URL, and verifies the endpoint proxies to the correct remote and returns 200.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import muxplex.settings as settings_mod
|
||||||
|
|
||||||
|
settings_path = tmp_path / "settings.json"
|
||||||
|
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||||
|
|
||||||
|
settings_path.write_text(
|
||||||
|
json.dumps(
|
||||||
|
{
|
||||||
|
"remote_instances": [
|
||||||
|
{
|
||||||
|
"url": "http://laptop:8088",
|
||||||
|
"key": "key-aaa",
|
||||||
|
"name": "Laptop",
|
||||||
|
"device_id": "aaa-111-bbb",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
post_calls = []
|
||||||
|
|
||||||
|
async def mock_post(url, **kwargs):
|
||||||
|
post_calls.append({"url": url, "kwargs": kwargs})
|
||||||
|
mock_resp = MagicMock()
|
||||||
|
mock_resp.status_code = 200
|
||||||
|
mock_resp.json.return_value = {
|
||||||
|
"active_session": "my-session",
|
||||||
|
"ttyd_port": 7682,
|
||||||
|
}
|
||||||
|
return mock_resp
|
||||||
|
|
||||||
|
mock_fed_client = MagicMock()
|
||||||
|
mock_fed_client.post = mock_post
|
||||||
|
monkeypatch.setattr(client.app.state, "federation_client", mock_fed_client)
|
||||||
|
|
||||||
|
response = client.post("/api/federation/aaa-111-bbb/connect/my-session")
|
||||||
|
assert response.status_code == 200, (
|
||||||
|
f"Expected 200, got {response.status_code}: {response.text}"
|
||||||
|
)
|
||||||
|
data = response.json()
|
||||||
|
assert data["active_session"] == "my-session"
|
||||||
|
|
||||||
|
|
||||||
|
def test_federation_connect_device_id_not_found(client, monkeypatch, tmp_path):
|
||||||
|
"""POST /api/federation/{device_id}/connect/{session_name} returns 404 when device_id has no match.
|
||||||
|
|
||||||
|
POSTs to a URL with an unrecognised device_id; the lookup returns None and
|
||||||
|
the endpoint must respond with HTTP 404.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
|
||||||
|
import muxplex.settings as settings_mod
|
||||||
|
|
||||||
|
settings_path = tmp_path / "settings.json"
|
||||||
|
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||||
|
|
||||||
|
# No remotes configured — any device_id lookup returns None
|
||||||
|
settings_path.write_text(json.dumps({"remote_instances": []}))
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
"/api/federation/nonexistent-device/connect/my-session"
|
||||||
|
)
|
||||||
|
assert response.status_code == 404, (
|
||||||
|
f"Expected 404 for unknown device_id, got {response.status_code}: {response.text}"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user