From 6d6b5351fabc2b4843bdd5a200f87cb301fffdc9 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 29 Aug 2026 01:00:12 +0800 Subject: [PATCH 1/4] fix: issue short-lived console tickets instead of JWTs in node_console MCP tool LLM clients transcribing the console WebSocket URL into shell commands reliably corrupted the ~200-char JWT embedded in it (dropped header segment -> "MissingAlgorithmError: Missing 'alg' value in header" on every connection attempt). The node_console tool now mints a short random ticket ("gns3t_" + 16 urlsafe chars, 10 min TTL, multi-use) stored server-side and bound to the node's console endpoints: - new ConsoleTicketService (gns3server/services/console_tickets.py), in-memory store with lazy expiry sweeps - get_current_active_user_from_websocket redeems tickets through the existing "token" query parameter, gated on websocket.path_params so a ticket only authenticates the console/ws and console/vnc routes of the node it was minted for; the JWT path is unchanged - redemption reuses the existing user lookup, token_version revocation and is_active checks, so logging out invalidates outstanding tickets - vnc_url no longer embeds the full session JWT - the tool docstring now tells clients to run the returned command verbatim instead of reconstructing the URL --- docs/features/mcp-service.md | 6 +- docs/features/vnc-websocket-console.md | 4 +- .../gns3_copilot/gns3_client/api_handlers.py | 31 ++- gns3server/agent/mcp/__init__.py | 24 +- .../controller/dependencies/authentication.py | 24 +- gns3server/services/__init__.py | 2 + gns3server/services/console_tickets.py | 115 +++++++++ tests/agent/mcp/test_handlers.py | 28 +++ .../routes/controller/test_console_tickets.py | 227 ++++++++++++++++++ 9 files changed, 431 insertions(+), 30 deletions(-) create mode 100644 gns3server/services/console_tickets.py create mode 100644 tests/api/routes/controller/test_console_tickets.py diff --git a/docs/features/mcp-service.md b/docs/features/mcp-service.md index 3dbfdbbd4..c3381343c 100644 --- a/docs/features/mcp-service.md +++ b/docs/features/mcp-service.md @@ -439,7 +439,7 @@ sequenceDiagram ### Console WebSocket -The `node_console` tool returns a WebSocket URL for connecting to a node's console. The URL includes a short-lived JWT (10 min) — reconnect if it expires. This endpoint is protocol-agnostic — it works for **telnet**, **ssh**, and **vnc** console types alike. The WebSocket simply proxies raw byte streams between the client and the compute node; protocol negotiation (e.g. SSH key exchange) happens on the compute side. +The `node_console` tool returns a WebSocket URL for connecting to a node's console. The URL includes a short-lived **console ticket** (10 min) — a short random string (`gns3t_…`, 22 chars) minted server-side and bound to that node's console endpoints. Tickets replaced the long JWT previously embedded here: LLM clients retyping the URL into shell commands reliably corrupted a ~200-char JWT, while a short ticket survives copying. Re-request the URL when the ticket expires (logging out also invalidates outstanding tickets). This endpoint is protocol-agnostic — it works for **telnet**, **ssh**, and **vnc** console types alike. The WebSocket simply proxies raw byte streams between the client and the compute node; protocol negotiation (e.g. SSH key exchange) happens on the compute side. The WebSocket URL is constructed using the server's `_server_url()`, which resolves the host as follows: @@ -454,11 +454,11 @@ When `Server.host` is `0.0.0.0` (listen on all interfaces), the MCP server disco If the configured host is already a specific IP or hostname (not `0.0.0.0`), it is used directly in the URL without modification. -Use `websocat` to connect from the command line: +Use `websocat` to connect from the command line (use the `command` returned by `node_console` verbatim — never reconstruct the URL by hand): ```bash # The host in the URL is automatically resolved to a reachable address -websocat ws://192.168.1.3:3080/v3/projects/{project_id}/nodes/{node_id}/console/ws?token= +websocat ws://192.168.1.3:3080/v3/projects/{project_id}/nodes/{node_id}/console/ws?token= ``` ### Source Files diff --git a/docs/features/vnc-websocket-console.md b/docs/features/vnc-websocket-console.md index 41b2a2ec2..b117ad152 100644 --- a/docs/features/vnc-websocket-console.md +++ b/docs/features/vnc-websocket-console.md @@ -58,7 +58,7 @@ graph LR **URL**: `ws://{controller_host}:{port}/v3/projects/{project_id}/nodes/{node_id}/console/vnc?token={jwt_token}` **Authentication**: -- JWT token via query parameter +- JWT token via query parameter, **or** a short-lived console ticket (`gns3t_…`, minted per node by the `node_console` MCP tool, valid 10 min, bound to this node's console endpoints) - User must have `Node.Console` privilege **WebSocket Subprotocols**: @@ -179,7 +179,7 @@ sequenceDiagram 1. **Authentication**: - JWT token validation via `has_privilege_on_websocket("Node.Console")` dependency - - Token passed as query parameter: `?token={jwt}` + - Token passed as query parameter: `?token={jwt}`, or a console ticket (`gns3t_…`) redeemable only on the node it was minted for 2. **Authorization**: - RBAC privilege check: `Node.Console` diff --git a/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py b/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py index 2dcec5afa..e8acdfc06 100644 --- a/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py +++ b/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py @@ -53,7 +53,8 @@ from concurrent.futures import ThreadPoolExecutor import hashlib import logging -from gns3server.services import auth_service +from gns3server.services import auth_service, console_ticket_service +from gns3server.services.console_tickets import DEFAULT_TICKET_TTL from gns3server.agent.gns3_copilot.gns3_client.connector import Gns3Connector @@ -416,12 +417,21 @@ def get_node_console_info_handler(params: dict[str, Any], gns3_ctx: dict[str, An node = conn.http_call("get", f"{conn.base_url}/projects/{project_id}/nodes/{node_id}").json() console_type = node.get("console_type", "unknown") - # Short-lived JWT for the WebSocket URL (10 min) + # Short-lived console ticket (10 min, multi-use, bound to this node's + # console endpoints). Deliberately a short random string instead of a JWT: + # LLM clients retype this URL into shell commands and reliably corrupted + # the ~200-char JWT previously embedded here (dropped header segment → + # "Missing 'alg' value in header" on the server). username = gns3_ctx.get("jwt_username") - ws_token = auth_service.create_access_token(username, token_version=gns3_ctx.get("jwt_token_version", 0), expires_in=10) if username else None + ticket = console_ticket_service.mint( + username, + token_version=gns3_ctx.get("jwt_token_version", 0), + project_id=project_id, + node_id=node_id, + ) if username else None raw_url = f"{gns3_ctx['server_url']}/v3/projects/{project_id}/nodes/{node_id}/console/ws" - if ws_token: - raw_url += f"?token={ws_token}" + if ticket: + raw_url += f"?token={ticket}" # Convert http scheme to ws for direct websocat usage ws_url = raw_url.replace("https://", "wss://").replace("http://", "ws://") @@ -432,14 +442,15 @@ def get_node_console_info_handler(params: dict[str, Any], gns3_ctx: dict[str, An "ws_url": ws_url, "command": f"websocat -t --no-close {ws_url}", } - if ws_token: + if ticket: # Fingerprint of the minted token: compare it against what actually reached the # server (logged on WebSocket auth rejection) to detect copy corruption, and # re-request the URL once token_ttl_seconds has elapsed. - result["token_sha256_prefix"] = hashlib.sha256(ws_token.encode()).hexdigest()[:8] - result["token_ttl_seconds"] = 600 - if console_type in ("vnc",): - result["vnc_url"] = f"/v3/projects/{project_id}/nodes/{node_id}/console/vnc?token={gns3_ctx['jwt_token']}" + result["token_sha256_prefix"] = hashlib.sha256(ticket.encode()).hexdigest()[:8] + result["token_ttl_seconds"] = DEFAULT_TICKET_TTL + if console_type in ("vnc",) and ticket: + # Same node binding covers the vnc endpoint (identical path params) + result["vnc_url"] = f"/v3/projects/{project_id}/nodes/{node_id}/console/vnc?token={ticket}" return result diff --git a/gns3server/agent/mcp/__init__.py b/gns3server/agent/mcp/__init__.py index a74031f5d..1caaa85fe 100644 --- a/gns3server/agent/mcp/__init__.py +++ b/gns3server/agent/mcp/__init__.py @@ -557,21 +557,19 @@ async def node_console( ) -> list[dict[str, Any]]: """Get WebSocket console connection info for a node. - Returns the WebSocket URL, console type (telnet/ssh/vnc), and other - connection details needed to interact with a node's console via WebSocket. - The URL includes a short-lived JWT (10 min) — reconnect if it expires. + Returns the console type (telnet/ssh/vnc) and a ready-to-run websocat + command with a short-lived access token (10 min) already embedded. - Complete workflow: - 1. Call this tool with project_id and node_id to get the WebSocket URL - 2. Connect to the returned URL using websocat in text mode (-t): - > websocat -t --no-close "ws://:3080/v3/projects/{project_id}/nodes/{node_id}/console/ws?token={jwt_token}" - 3. Send device commands with \\r\\n line endings via heredoc: - > websocat -t --no-close "ws://..." <<< $'\\r\\nenable\\r\\nshow version\\r\\nexit\\r\\n' - 4. Receive response: websocat receives and displays device output - Use 'timeout' to avoid connection hanging: - > timeout 10 websocat -t --no-close "ws://..." <<< $'commands\\r\\n' + IMPORTANT — copy the returned values EXACTLY: + - Run the returned "command" string verbatim; it already contains the + full URL with token. NEVER construct or edit the URL yourself, and + NEVER copy the token by hand — a mistyped token is rejected. + - The token expires after token_ttl_seconds (10 min): call this tool + again to get a fresh one; do not reuse an old URL. + + Sending device commands after connecting: + > timeout 10 websocat -t --no-close "ws://..." <<< $'\\r\\nenable\\r\\nshow version\\r\\nexit\\r\\n' - Key points: - Use \\r\\n (not \\n) to match console protocol line endings - Use $'...' format for escape sequences in bash - --no-close keeps the WebSocket open after stdin (heredoc) hits EOF, so diff --git a/gns3server/api/routes/controller/dependencies/authentication.py b/gns3server/api/routes/controller/dependencies/authentication.py index 1712155cd..7ff79cb4f 100644 --- a/gns3server/api/routes/controller/dependencies/authentication.py +++ b/gns3server/api/routes/controller/dependencies/authentication.py @@ -29,7 +29,9 @@ import gns3server.db.models as models from gns3server.db.repositories.api_keys import ApiKeysRepository from gns3server.db.repositories.users import UsersRepository from gns3server.db.repositories.rbac import RbacRepository -from gns3server.services import auth_service +from gns3server.schemas.controller.tokens import TokenData +from gns3server.services import auth_service, console_ticket_service +from gns3server.services.console_tickets import TICKET_PREFIX from .database import get_repository log = logging.getLogger(__name__) @@ -149,7 +151,25 @@ async def get_current_active_user_from_websocket( await websocket.accept(subprotocol=subprotocol) try: - token_data = auth_service.get_token_data(token) + if token.startswith(TICKET_PREFIX): + # Console tickets ("gns3t_…"): short-lived credentials bound to one + # node's console endpoints, minted by the node_console MCP tool. + # redeem() confines them to the route matching their binding, so + # they cannot authenticate the notification/wireshark sockets that + # share this dependency. + ticket = console_ticket_service.redeem(token, websocket.path_params) + if ticket is None: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid or expired console ticket", + ) + token_data = TokenData( + username=ticket.username, + token_version=ticket.token_version, + token_use="access", + ) + else: + token_data = auth_service.get_token_data(token) _reject_refresh_token(token_data) user = await user_repo.get_user_by_username(token_data.username) diff --git a/gns3server/services/__init__.py b/gns3server/services/__init__.py index f9be542e2..f44e732a2 100644 --- a/gns3server/services/__init__.py +++ b/gns3server/services/__init__.py @@ -15,5 +15,7 @@ # along with this program. If not, see . from .authentication import AuthService +from .console_tickets import ConsoleTicketService auth_service = AuthService() +console_ticket_service = ConsoleTicketService() diff --git a/gns3server/services/console_tickets.py b/gns3server/services/console_tickets.py new file mode 100644 index 000000000..6f238ac28 --- /dev/null +++ b/gns3server/services/console_tickets.py @@ -0,0 +1,115 @@ +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +""" +Short-lived console tickets. + +A console ticket is a short random string ("gns3t_" + 16 urlsafe chars) that +replaces the long JWT previously embedded in console WebSocket URLs returned +to LLM clients, which reliably corrupted the ~200-char JWT when retyping it +into a shell command. The ticket carries no information itself — the server +remembers what it maps to, which is what makes a 96-bit random string a +sufficient credential. + +Tickets are bound to a single node's console endpoints (console/ws and +console/vnc share the same binding) and are multi-use within their TTL, so +console clients that reconnect after a drop keep working. Redeeming also +re-checks the minting user's token_version, so logging out invalidates +outstanding tickets just like it invalidates JWTs. +""" + +import logging +import secrets +import time +from dataclasses import dataclass +from typing import Optional + +log = logging.getLogger(__name__) + +# Distinguishes tickets from JWTs (which contain dots) and API keys ("gns3_") +TICKET_PREFIX = "gns3t_" +DEFAULT_TICKET_TTL = 600 # seconds + + +@dataclass +class ConsoleTicket: + username: str + token_version: int + project_id: str + node_id: str + expires_at: float # time.monotonic() based + + +class ConsoleTicketService: + """ + In-memory store for console tickets. + + Single-process asyncio app: minting (MCP tool handlers, via worker + threads) and redemption (WS auth dependency, event loop) share one dict. + dict get/set/del are atomic under the GIL and keys are random, so no + locking is needed. Tickets vanish on restart — clients just request a + new one. + """ + + def __init__(self) -> None: + self._tickets: dict[str, ConsoleTicket] = {} + + def mint( + self, + username: str, + token_version: int, + project_id: str, + node_id: str, + ttl: int = DEFAULT_TICKET_TTL, + ) -> str: + # 12 random bytes → 16 urlsafe chars (~96 bits); comfortably + # unguessable within the 10-minute window and short enough that + # LLM clients copy it without corruption. + ticket = TICKET_PREFIX + secrets.token_urlsafe(12) + self._sweep_expired() + self._tickets[ticket] = ConsoleTicket( + username=username, + token_version=token_version, + project_id=project_id, + node_id=node_id, + expires_at=time.monotonic() + ttl, + ) + return ticket + + def redeem(self, ticket: str, path_params: dict) -> Optional[ConsoleTicket]: + """ + Validate a ticket against the route it is being used on. + + Returns the ticket record if valid, None otherwise. The route's path + parameters must match the ticket's binding, which confines a ticket + to the node's console endpoints — routes without a node_id path + parameter (notifications, web wireshark, …) always fail the binding. + """ + + record = self._tickets.get(ticket) + if record is None: + return None + if time.monotonic() >= record.expires_at: + self._tickets.pop(ticket, None) + return None + if path_params.get("project_id") != record.project_id or path_params.get("node_id") != record.node_id: + return None + return record + + def _sweep_expired(self) -> None: + now = time.monotonic() + for ticket in [t for t, record in self._tickets.items() if now >= record.expires_at]: + del self._tickets[ticket] diff --git a/tests/agent/mcp/test_handlers.py b/tests/agent/mcp/test_handlers.py index b71b33ee0..2588e98c3 100644 --- a/tests/agent/mcp/test_handlers.py +++ b/tests/agent/mcp/test_handlers.py @@ -280,10 +280,38 @@ class TestNode: def test_console(self, ctx): from gns3server.agent.gns3_copilot.gns3_client.api_handlers import get_node_console_info_handler + from gns3server.services import console_ticket_service with patch(f"{AH}._get_connector") as m: m.return_value = _mock_conn({"console_url": "ws://host/console"}) result = get_node_console_info_handler({"project_id": "p1", "node_id": "n1"}, ctx) assert "command" in result + # the URL embeds a short console ticket (not a JWT) redeemable for this node + assert "?token=gns3t_" in result["ws_url"] + ticket = result["ws_url"].split("token=")[1] + assert ticket in result["command"] + assert result["token_ttl_seconds"] == 600 + assert len(result["token_sha256_prefix"]) == 8 + redeemed = console_ticket_service.redeem(ticket, {"project_id": "p1", "node_id": "n1"}) + assert redeemed is not None and redeemed.username == "admin" + assert "vnc_url" not in result # console_type is not vnc + + def test_console_vnc_uses_same_ticket(self, ctx): + from gns3server.agent.gns3_copilot.gns3_client.api_handlers import get_node_console_info_handler + with patch(f"{AH}._get_connector") as m: + m.return_value = _mock_conn({"console_type": "vnc"}) + result = get_node_console_info_handler({"project_id": "p1", "node_id": "n1"}, ctx) + ticket = result["ws_url"].split("token=")[1] + # one node-bound ticket covers both console endpoints (identical path params) + assert result["vnc_url"] == f"/v3/projects/p1/nodes/n1/console/vnc?token={ticket}" + + def test_console_without_username_omits_token(self, ctx): + from gns3server.agent.gns3_copilot.gns3_client.api_handlers import get_node_console_info_handler + unauthenticated_ctx = {k: v for k, v in ctx.items() if k != "jwt_username"} + with patch(f"{AH}._get_connector") as m: + m.return_value = _mock_conn({"console_url": "ws://host/console"}) + result = get_node_console_info_handler({"project_id": "p1", "node_id": "n1"}, unauthenticated_ctx) + assert "token=" not in result["ws_url"] + assert "token_sha256_prefix" not in result @staticmethod def _file_conn(text): diff --git a/tests/api/routes/controller/test_console_tickets.py b/tests/api/routes/controller/test_console_tickets.py new file mode 100644 index 000000000..4518d26d8 --- /dev/null +++ b/tests/api/routes/controller/test_console_tickets.py @@ -0,0 +1,227 @@ +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import hashlib +from types import SimpleNamespace +from typing import List + +import aiohttp +import pytest +from fastapi import FastAPI +from httpx import AsyncClient +from httpx_ws import aconnect_ws +from httpx_ws.transport import ASGIWebSocketTransport +from pydantic import SecretStr + +from gns3server.config import Config +from gns3server.controller import Controller +from gns3server.controller.compute import Compute +from gns3server.controller.node import Node +from gns3server.controller.project import Project +from gns3server.services.console_tickets import ( + ConsoleTicketService, + DEFAULT_TICKET_TTL, + TICKET_PREFIX, +) +from gns3server.services import console_ticket_service +from gns3server.utils.http_client import HTTPClient +from tests.api.routes.controller.test_nodes import FakeComputeConsoleWebSocket + + +class TestConsoleTicketService: + """Unit tests for the in-memory ticket store (fresh instances, no app).""" + + def test_mint_format(self) -> None: + + service = ConsoleTicketService() + ticket = service.mint("admin", 1, "p1", "n1") + assert ticket.startswith(TICKET_PREFIX) + # 12 random bytes -> 16 urlsafe chars; shell-safe alphabet (no quoting hazards) + assert len(ticket) == len(TICKET_PREFIX) + 16 + assert "." not in ticket # never confusable with a JWT + assert service.mint("admin", 1, "p1", "n1") != ticket + + def test_redeem_valid_ticket_is_multi_use(self) -> None: + + service = ConsoleTicketService() + ticket = service.mint("admin", 7, "p1", "n1") + path_params = {"project_id": "p1", "node_id": "n1"} + first = service.redeem(ticket, path_params) + second = service.redeem(ticket, path_params) # console clients reconnect, tickets stay usable + assert first is not None and second is not None + assert first.username == "admin" + assert first.token_version == 7 + + def test_redeem_rejects_wrong_binding(self) -> None: + + service = ConsoleTicketService() + ticket = service.mint("admin", 0, "p1", "n1") + assert service.redeem(ticket, {"project_id": "p1", "node_id": "n2"}) is None + assert service.redeem(ticket, {"project_id": "p2", "node_id": "n1"}) is None + # routes without a node_id path parameter (notifications, wireshark…) never accept a ticket + assert service.redeem(ticket, {"project_id": "p1"}) is None + assert service.redeem(ticket, {}) is None + + def test_redeem_rejects_unknown_ticket(self) -> None: + + service = ConsoleTicketService() + assert service.redeem(TICKET_PREFIX + "doesnotexist", {"project_id": "p1", "node_id": "n1"}) is None + + def test_expired_ticket_is_rejected_and_removed(self) -> None: + + service = ConsoleTicketService() + ticket = service.mint("admin", 0, "p1", "n1", ttl=0) + assert service.redeem(ticket, {"project_id": "p1", "node_id": "n1"}) is None + assert ticket not in service._tickets + + def test_mint_sweeps_expired_entries(self) -> None: + + service = ConsoleTicketService() + stale = service.mint("admin", 0, "p1", "n1", ttl=0) + fresh = service.mint("admin", 0, "p1", "n2", ttl=DEFAULT_TICKET_TTL) + assert stale not in service._tickets + assert fresh in service._tickets + + +class TestConsoleTicketWebSocketAuth: + """ + Drive the console WebSocket auth dependency end-to-end through the real + routes (the shared service singleton is the one the dependency consults). + """ + + pytestmark = pytest.mark.asyncio + + @pytest.fixture(autouse=True) + def _reset_controller_singleton(self, controller): + # The project/compute fixtures register state on the shared Controller + # singleton; reset it after each test so files running later (e.g. the + # controller statistics endpoint) see a pristine controller regardless + # of test order — same reset the controller fixture applies at setup. + yield + Controller._instance = None + + @pytest.fixture + def node(self, project: Project, compute: Compute) -> Node: + + compute.host = "127.0.0.1" + compute.port = 3080 + node = Node(project, compute, "test", node_type="vpcs") + project._nodes[node.id] = node + return node + + @pytest.fixture + def compute_credentials(self) -> None: + + server_config = Config.instance().settings.Server + server_config.compute_username = "admin" + server_config.compute_password = SecretStr("password") + + @staticmethod + def _ws_client(app: FastAPI, base_client: AsyncClient) -> AsyncClient: + # base_client must be requested so the app gets the test-DB dependency + # override, but the WebSocket connection itself needs a function-local + # client: closing the WS transport from the class-scoped base_client + # teardown exits anyio cancel scopes in the wrong task. + return AsyncClient(base_url="http://test-api", transport=ASGIWebSocketTransport(app=app)) + + @staticmethod + def _forward_compute_ws(monkeypatch, messages: List[aiohttp.WSMessage]) -> FakeComputeConsoleWebSocket: + + compute_ws = FakeComputeConsoleWebSocket(messages) + monkeypatch.setattr( + HTTPClient, + "get_client", + classmethod(lambda cls: SimpleNamespace(ws_connect=lambda *args, **kwargs: compute_ws)) + ) + return compute_ws + + async def test_console_ws_accepts_valid_ticket( + self, + app: FastAPI, + base_client: AsyncClient, + compute_credentials, + project: Project, + node: Node, + monkeypatch + ) -> None: + # admin is the seeded superadmin, so the RBAC privilege check is skipped + ticket = console_ticket_service.mint("admin", 0, project.id, node.id) + self._forward_compute_ws(monkeypatch, [ + aiohttp.WSMessage(aiohttp.WSMsgType.TEXT, "device output", None), + ]) + + async with self._ws_client(app, base_client) as client: + async with aconnect_ws( + f"/v3/projects/{project.id}/nodes/{node.id}/console/ws", + client, + params={"token": ticket} + ) as ws: + # reaching the compute forwarding loop means ticket auth succeeded + assert await ws.receive_text() == "device output" + + async def test_console_ws_rejects_ticket_bound_to_other_node( + self, + app: FastAPI, + base_client: AsyncClient, + project: Project, + node: Node + ) -> None: + + other_node = "00000000-0000-0000-0000-000000000000" + ticket = console_ticket_service.mint("admin", 0, project.id, other_node) + async with self._ws_client(app, base_client) as client: + async with aconnect_ws( + f"/v3/projects/{project.id}/nodes/{node.id}/console/ws", + client, + params={"token": ticket} + ) as ws: + notification = await ws.receive_json() + assert notification["event"]["message"] == ( + "Could not authenticate while connecting to controller WebSocket: " + "Invalid or expired console ticket " + f"(received token sha256 prefix: {hashlib.sha256(ticket.encode()).hexdigest()[:8]})" + ) + + async def test_console_ws_rejects_ticket_with_stale_token_version( + self, + app: FastAPI, + base_client: AsyncClient, + project: Project, + node: Node + ) -> None: + # logging out bumps the user's token_version: outstanding tickets must die with it + ticket = console_ticket_service.mint("admin", 999, project.id, node.id) + async with self._ws_client(app, base_client) as client: + async with aconnect_ws( + f"/v3/projects/{project.id}/nodes/{node.id}/console/ws", + client, + params={"token": ticket} + ) as ws: + notification = await ws.receive_json() + assert "Token has been revoked for 'admin'" in notification["event"]["message"] + + async def test_ticket_rejected_on_non_console_websocket( + self, + app: FastAPI, + base_client: AsyncClient + ) -> None: + # the controller notification stream shares the WS auth dependency but has + # no node binding: a console ticket must not authenticate it + ticket = console_ticket_service.mint("admin", 0, "p1", "n1") + async with self._ws_client(app, base_client) as client: + async with aconnect_ws("/v3/notifications/ws", client, params={"token": ticket}) as ws: + notification = await ws.receive_json() + assert "Invalid or expired console ticket" in notification["event"]["message"] From d1edfbe5e8f76fb8ff7dfbb9cdcf167896411798 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 29 Aug 2026 01:10:19 +0800 Subject: [PATCH 2/4] fix: replace Bearer JWTs with path-bound access tickets in MCP download tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit link_capture_download and get_symbol embedded a 10-min JWT in the Authorization header of the curl command they return; LLM clients retyping that command corrupted the long token — the same failure class as the console WebSocket URLs fixed in the previous commit. Generalize the ticket store (console_tickets.py -> access_tickets.py, ConsoleTicketService -> AccessTicketService): a ticket now binds to either a node's console endpoints (WebSocket, matched against route path params) or one exact REST resource path (capture file, symbol image). The two binding modes are isolated — a node-bound ticket cannot authenticate a REST resource and vice versa. get_user_from_token redeems path-bound tickets through the existing token parameter / Bearer header, matched exactly against request.url.path, then reuses the shared user lookup and token_version revocation checks. Download URLs embed ?token= and the curl commands no longer carry a Bearer header. --- docs/features/mcp-service.md | 2 +- .../gns3_copilot/gns3_client/api_handlers.py | 46 ++++--- gns3server/agent/mcp/symbols.py | 19 ++- .../controller/dependencies/authentication.py | 49 +++++-- gns3server/services/__init__.py | 4 +- .../{console_tickets.py => access_tickets.py} | 82 ++++++++---- tests/agent/mcp/test_handlers.py | 54 +++++++- ...sole_tickets.py => test_access_tickets.py} | 125 ++++++++++++++---- 8 files changed, 293 insertions(+), 88 deletions(-) rename gns3server/services/{console_tickets.py => access_tickets.py} (57%) rename tests/api/routes/controller/{test_console_tickets.py => test_access_tickets.py} (61%) diff --git a/docs/features/mcp-service.md b/docs/features/mcp-service.md index c3381343c..a6d5eb837 100644 --- a/docs/features/mcp-service.md +++ b/docs/features/mcp-service.md @@ -439,7 +439,7 @@ sequenceDiagram ### Console WebSocket -The `node_console` tool returns a WebSocket URL for connecting to a node's console. The URL includes a short-lived **console ticket** (10 min) — a short random string (`gns3t_…`, 22 chars) minted server-side and bound to that node's console endpoints. Tickets replaced the long JWT previously embedded here: LLM clients retyping the URL into shell commands reliably corrupted a ~200-char JWT, while a short ticket survives copying. Re-request the URL when the ticket expires (logging out also invalidates outstanding tickets). This endpoint is protocol-agnostic — it works for **telnet**, **ssh**, and **vnc** console types alike. The WebSocket simply proxies raw byte streams between the client and the compute node; protocol negotiation (e.g. SSH key exchange) happens on the compute side. +The `node_console` tool returns a WebSocket URL for connecting to a node's console. The URL includes a short-lived **access ticket** (10 min) — a short random string (`gns3t_…`, 22 chars) minted server-side and bound to that node's console endpoints. `link_capture_download` uses the same kind of ticket bound to one exact resource path instead of embedding a Bearer JWT in the returned curl command. Tickets replaced the long JWTs previously embedded in these URLs/commands: LLM clients retyping them into shell commands reliably corrupted a ~200-char JWT, while a short ticket survives copying. Re-request the URL when the ticket expires (logging out also invalidates outstanding tickets). This endpoint is protocol-agnostic — it works for **telnet**, **ssh**, and **vnc** console types alike. The WebSocket simply proxies raw byte streams between the client and the compute node; protocol negotiation (e.g. SSH key exchange) happens on the compute side. The WebSocket URL is constructed using the server's `_server_url()`, which resolves the host as follows: diff --git a/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py b/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py index e8acdfc06..831dace7b 100644 --- a/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py +++ b/gns3server/agent/gns3_copilot/gns3_client/api_handlers.py @@ -53,8 +53,8 @@ from concurrent.futures import ThreadPoolExecutor import hashlib import logging -from gns3server.services import auth_service, console_ticket_service -from gns3server.services.console_tickets import DEFAULT_TICKET_TTL +from gns3server.services import access_ticket_service +from gns3server.services.access_tickets import DEFAULT_TICKET_TTL from gns3server.agent.gns3_copilot.gns3_client.connector import Gns3Connector @@ -423,7 +423,7 @@ def get_node_console_info_handler(params: dict[str, Any], gns3_ctx: dict[str, An # the ~200-char JWT previously embedded here (dropped header segment → # "Missing 'alg' value in header" on the server). username = gns3_ctx.get("jwt_username") - ticket = console_ticket_service.mint( + ticket = access_ticket_service.mint( username, token_version=gns3_ctx.get("jwt_token_version", 0), project_id=project_id, @@ -859,34 +859,36 @@ def download_capture_file_handler(params: dict[str, Any], gns3_ctx: dict[str, An if not project_id: return {"error": "project_id is required"} username = gns3_ctx.get("jwt_username") - download_token = auth_service.create_access_token(username, token_version=gns3_ctx.get("jwt_token_version", 0), expires_in=10) if username else None + token_version = gns3_ctx.get("jwt_token_version", 0) + + def _download(link_id: str) -> dict[str, Any]: + # short-lived ticket bound to this exact download path — LLM clients + # retyping curl commands corrupted the long Bearer JWT this used to embed + path = f"/v3/projects/{project_id}/links/{link_id}/capture/file" + url = f"{gns3_ctx['server_url']}{path}" + ticket = access_ticket_service.mint(username, token_version=token_version, path=path) if username else None + if ticket: + url += f"?token={ticket}" + entry = {"link_id": link_id, "download_url": url} + if ticket: + entry["curl_command"] = f"curl -L -o capture_{link_id}.pcap '{url}'" + return entry link_ids = params.get("link_ids") if link_ids: if not isinstance(link_ids, list): return {"error": "link_ids must be a list"} - results = [] - for lid in link_ids: - url = f"{gns3_ctx['server_url']}/v3/projects/{project_id}/links/{lid}/capture/file" - entry = {"link_id": lid, "download_url": url} - if download_token: - cmd = f"curl -L -o capture_{lid}.pcap -H 'Authorization: Bearer {download_token}' '{url}'" - entry["curl_command"] = cmd - results.append(entry) - return {"downloads": results, "count": len(results), "note": "Files are in pcap format. Links include a 10-minute token."} + results = [_download(lid) for lid in link_ids] + return {"downloads": results, "count": len(results), "note": "Files are in pcap format. URLs include a 10-minute ticket."} link_id = params.get("link_id") if not link_id: return {"error": "link_id or link_ids is required"} - download_url = f"{gns3_ctx['server_url']}/v3/projects/{project_id}/links/{link_id}/capture/file" - result = { - "link_id": link_id, - "download_url": download_url, - "note": "The file is in pcap format and can be analyzed with Wireshark or tcpdump.", - } - if download_token: - result["curl_command"] = f"curl -L -o capture.pcap -H 'Authorization: Bearer {download_token}' '{download_url}'" - result["note"] += " The download link includes a 10-minute token." + result = _download(link_id) + result["note"] = "The file is in pcap format and can be analyzed with Wireshark or tcpdump." + if "curl_command" in result: + result["curl_command"] = f"curl -L -o capture.pcap '{result['download_url']}'" + result["note"] += " The download URL includes a 10-minute ticket." return result diff --git a/gns3server/agent/mcp/symbols.py b/gns3server/agent/mcp/symbols.py index 8625d3e0c..96f2841d6 100644 --- a/gns3server/agent/mcp/symbols.py +++ b/gns3server/agent/mcp/symbols.py @@ -23,7 +23,7 @@ from typing import Any import logging -from gns3server.services import auth_service +from gns3server.services import access_ticket_service log = logging.getLogger(__name__) @@ -52,18 +52,25 @@ def get_symbol_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict symbol_id = params.get("symbol_id") if not symbol_id: return {"error": "symbol_id is required"} - download_url = f"{gns3_ctx['server_url']}/v3/symbols/{symbol_id}/raw" + path = f"/v3/symbols/{symbol_id}/raw" + download_url = f"{gns3_ctx['server_url']}{path}" username = gns3_ctx.get("jwt_username") - download_token = auth_service.create_access_token(username, token_version=gns3_ctx.get("jwt_token_version", 0), expires_in=10) if username else None + # short-lived ticket bound to this exact path — LLM clients retyping curl + # commands corrupted the long Bearer JWT this used to embed + ticket = access_ticket_service.mint( + username, token_version=gns3_ctx.get("jwt_token_version", 0), path=path + ) if username else None + if ticket: + download_url += f"?token={ticket}" result = { "symbol_id": symbol_id, "download_url": download_url, "note": "Symbol files are SVG images.", } - if download_token: + if ticket: safe_name = symbol_id.replace(':', '').replace('/', '_') - result["curl_command"] = f"curl -L -o '{safe_name}.svg' -H 'Authorization: Bearer {download_token}' '{download_url}'" - result["note"] += " Download link includes a 10-minute token." + result["curl_command"] = f"curl -L -o '{safe_name}.svg' '{download_url}'" + result["note"] += " The download URL includes a 10-minute ticket." return result diff --git a/gns3server/api/routes/controller/dependencies/authentication.py b/gns3server/api/routes/controller/dependencies/authentication.py index 7ff79cb4f..d363c996e 100644 --- a/gns3server/api/routes/controller/dependencies/authentication.py +++ b/gns3server/api/routes/controller/dependencies/authentication.py @@ -30,8 +30,8 @@ from gns3server.db.repositories.api_keys import ApiKeysRepository from gns3server.db.repositories.users import UsersRepository from gns3server.db.repositories.rbac import RbacRepository from gns3server.schemas.controller.tokens import TokenData -from gns3server.services import auth_service, console_ticket_service -from gns3server.services.console_tickets import TICKET_PREFIX +from gns3server.services import auth_service, access_ticket_service +from gns3server.services.access_tickets import TICKET_PREFIX from .database import get_repository log = logging.getLogger(__name__) @@ -50,6 +50,7 @@ def _reject_refresh_token(token_data) -> None: async def get_user_from_token( + request: Request, bearer_token: str = Depends(oauth2_scheme), user_repo: UsersRepository = Depends(get_repository(UsersRepository)), api_keys_repo: ApiKeysRepository = Depends(get_repository(ApiKeysRepository)), @@ -68,6 +69,38 @@ async def get_user_from_token( headers={"WWW-Authenticate": "Bearer"}, ) + if token.startswith(TICKET_PREFIX): + # Access tickets ("gns3t_…"): short-lived credentials bound to one + # exact resource path, minted by the MCP download tools (capture + # files, symbols). redeem_for_path() confines the ticket to that + # path, so it cannot be replayed against any other resource. + ticket = access_ticket_service.redeem_for_path(token, request.url.path) + if ticket is None: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid or expired access ticket", + headers={"WWW-Authenticate": "Bearer"}, + ) + token_data = TokenData( + username=ticket.username, + token_version=ticket.token_version, + token_use="access", + ) + user = await user_repo.get_user_by_username(token_data.username) + if user is None: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Could not validate credentials", + headers={"WWW-Authenticate": "Bearer"}, + ) + if token_data.token_version != user.token_version: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail=f"Token has been revoked for '{token_data.username}'", + headers={"WWW-Authenticate": "Bearer"}, + ) + return user + # API Key authentication — format: gns3__ # Direct lookup by UUID avoids O(n) scan of all keys. if token.startswith("gns3_"): @@ -152,12 +185,12 @@ async def get_current_active_user_from_websocket( try: if token.startswith(TICKET_PREFIX): - # Console tickets ("gns3t_…"): short-lived credentials bound to one - # node's console endpoints, minted by the node_console MCP tool. - # redeem() confines them to the route matching their binding, so - # they cannot authenticate the notification/wireshark sockets that - # share this dependency. - ticket = console_ticket_service.redeem(token, websocket.path_params) + # Node-bound access tickets ("gns3t_…"): short-lived credentials + # for one node's console endpoints, minted by the node_console + # MCP tool. redeem() confines them to the route matching their + # binding, so they cannot authenticate the notification/wireshark + # sockets that share this dependency. + ticket = access_ticket_service.redeem(token, websocket.path_params) if ticket is None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/gns3server/services/__init__.py b/gns3server/services/__init__.py index f44e732a2..572a235a2 100644 --- a/gns3server/services/__init__.py +++ b/gns3server/services/__init__.py @@ -15,7 +15,7 @@ # along with this program. If not, see . from .authentication import AuthService -from .console_tickets import ConsoleTicketService +from .access_tickets import AccessTicketService auth_service = AuthService() -console_ticket_service = ConsoleTicketService() +access_ticket_service = AccessTicketService() diff --git a/gns3server/services/console_tickets.py b/gns3server/services/access_tickets.py similarity index 57% rename from gns3server/services/console_tickets.py rename to gns3server/services/access_tickets.py index 6f238ac28..287e4df5c 100644 --- a/gns3server/services/console_tickets.py +++ b/gns3server/services/access_tickets.py @@ -15,20 +15,27 @@ # along with this program. If not, see . """ -Short-lived console tickets. +Short-lived access tickets. -A console ticket is a short random string ("gns3t_" + 16 urlsafe chars) that -replaces the long JWT previously embedded in console WebSocket URLs returned +An access ticket is a short random string ("gns3t_" + 16 urlsafe chars) that +replaces the long JWTs previously embedded in URLs and curl commands returned to LLM clients, which reliably corrupted the ~200-char JWT when retyping it into a shell command. The ticket carries no information itself — the server remembers what it maps to, which is what makes a 96-bit random string a sufficient credential. -Tickets are bound to a single node's console endpoints (console/ws and -console/vnc share the same binding) and are multi-use within their TTL, so -console clients that reconnect after a drop keep working. Redeeming also -re-checks the minting user's token_version, so logging out invalidates -outstanding tickets just like it invalidates JWTs. +A ticket is bound to exactly one target, in one of two modes: + +- node binding (project_id + node_id): redeemable on that node's console + WebSocket endpoints (console/ws and console/vnc share the binding), + validated against the route's path parameters. +- path binding (exact resource path): redeemable on one REST resource path + (e.g. a capture file download or a symbol image), validated against + request.url.path. + +Tickets are multi-use within their TTL, so clients that reconnect keep +working. Redemption re-checks the minting user's token_version, so logging +out invalidates outstanding tickets just like it invalidates JWTs. """ import logging @@ -45,34 +52,36 @@ DEFAULT_TICKET_TTL = 600 # seconds @dataclass -class ConsoleTicket: +class AccessTicket: username: str token_version: int - project_id: str - node_id: str - expires_at: float # time.monotonic() based + project_id: Optional[str] = None # node binding: console WebSocket endpoints + node_id: Optional[str] = None + path: Optional[str] = None # path binding: one exact REST resource path + expires_at: float = 0.0 # time.monotonic() based -class ConsoleTicketService: +class AccessTicketService: """ - In-memory store for console tickets. + In-memory store for access tickets. Single-process asyncio app: minting (MCP tool handlers, via worker - threads) and redemption (WS auth dependency, event loop) share one dict. + threads) and redemption (auth dependencies, event loop) share one dict. dict get/set/del are atomic under the GIL and keys are random, so no locking is needed. Tickets vanish on restart — clients just request a new one. """ def __init__(self) -> None: - self._tickets: dict[str, ConsoleTicket] = {} + self._tickets: dict[str, AccessTicket] = {} def mint( self, username: str, token_version: int, - project_id: str, - node_id: str, + project_id: Optional[str] = None, + node_id: Optional[str] = None, + path: Optional[str] = None, ttl: int = DEFAULT_TICKET_TTL, ) -> str: # 12 random bytes → 16 urlsafe chars (~96 bits); comfortably @@ -80,33 +89,58 @@ class ConsoleTicketService: # LLM clients copy it without corruption. ticket = TICKET_PREFIX + secrets.token_urlsafe(12) self._sweep_expired() - self._tickets[ticket] = ConsoleTicket( + self._tickets[ticket] = AccessTicket( username=username, token_version=token_version, project_id=project_id, node_id=node_id, + path=path, expires_at=time.monotonic() + ttl, ) return ticket - def redeem(self, ticket: str, path_params: dict) -> Optional[ConsoleTicket]: + def redeem(self, ticket: str, path_params: dict) -> Optional[AccessTicket]: """ - Validate a ticket against the route it is being used on. + Validate a node-bound ticket against a WebSocket route. Returns the ticket record if valid, None otherwise. The route's path parameters must match the ticket's binding, which confines a ticket to the node's console endpoints — routes without a node_id path - parameter (notifications, web wireshark, …) always fail the binding. + parameter (notifications, web wireshark, …) always fail the binding, + and so do path-bound (REST) tickets. """ + record = self._get_valid(ticket) + if record is None: + return None + if record.node_id is None or record.path is not None: + return None + if path_params.get("project_id") != record.project_id or path_params.get("node_id") != record.node_id: + return None + return record + + def redeem_for_path(self, ticket: str, path: str) -> Optional[AccessTicket]: + """ + Validate a path-bound ticket against a REST resource path. + + The path must match exactly — a ticket minted for one capture file + download cannot be replayed against any other resource. + """ + + record = self._get_valid(ticket) + if record is None: + return None + if record.path is None or record.path != path: + return None + return record + + def _get_valid(self, ticket: str) -> Optional[AccessTicket]: record = self._tickets.get(ticket) if record is None: return None if time.monotonic() >= record.expires_at: self._tickets.pop(ticket, None) return None - if path_params.get("project_id") != record.project_id or path_params.get("node_id") != record.node_id: - return None return record def _sweep_expired(self) -> None: diff --git a/tests/agent/mcp/test_handlers.py b/tests/agent/mcp/test_handlers.py index 2588e98c3..87501bf30 100644 --- a/tests/agent/mcp/test_handlers.py +++ b/tests/agent/mcp/test_handlers.py @@ -280,7 +280,7 @@ class TestNode: def test_console(self, ctx): from gns3server.agent.gns3_copilot.gns3_client.api_handlers import get_node_console_info_handler - from gns3server.services import console_ticket_service + from gns3server.services import access_ticket_service with patch(f"{AH}._get_connector") as m: m.return_value = _mock_conn({"console_url": "ws://host/console"}) result = get_node_console_info_handler({"project_id": "p1", "node_id": "n1"}, ctx) @@ -291,7 +291,7 @@ class TestNode: assert ticket in result["command"] assert result["token_ttl_seconds"] == 600 assert len(result["token_sha256_prefix"]) == 8 - redeemed = console_ticket_service.redeem(ticket, {"project_id": "p1", "node_id": "n1"}) + redeemed = access_ticket_service.redeem(ticket, {"project_id": "p1", "node_id": "n1"}) assert redeemed is not None and redeemed.username == "admin" assert "vnc_url" not in result # console_type is not vnc @@ -462,6 +462,56 @@ class TestLink: }, ctx) assert result["suspend"] is True + def test_download_capture_file(self, ctx): + from gns3server.agent.gns3_copilot.gns3_client.api_handlers import download_capture_file_handler + from gns3server.services import access_ticket_service + result = download_capture_file_handler({"project_id": "p1", "link_id": "l1"}, ctx) + # a short path-bound ticket replaces the long Bearer JWT in the URL + assert "?token=gns3t_" in result["download_url"] + assert "Bearer" not in result["curl_command"] + path = f"/v3/projects/p1/links/l1/capture/file" + ticket = result["download_url"].split("token=")[1] + assert result["curl_command"] == f"curl -L -o capture.pcap '{result['download_url']}'" + redeemed = access_ticket_service.redeem_for_path(ticket, path) + assert redeemed is not None and redeemed.username == "admin" + + def test_download_capture_file_batch(self, ctx): + from gns3server.agent.gns3_copilot.gns3_client.api_handlers import download_capture_file_handler + from gns3server.services import access_ticket_service + result = download_capture_file_handler({"project_id": "p1", "link_ids": ["l1", "l2"]}, ctx) + assert result["count"] == 2 + # each link gets its own ticket bound to its own download path + tickets = [entry["download_url"].split("token=")[1] for entry in result["downloads"]] + assert tickets[0] != tickets[1] + for lid, ticket in zip(["l1", "l2"], tickets): + redeemed = access_ticket_service.redeem_for_path(ticket, f"/v3/projects/p1/links/{lid}/capture/file") + assert redeemed is not None + + +# ── Symbol ────────────────────────────────────────────────────────────── + + +class TestSymbol: + + mod = "symbols" + + def test_get_symbol_download(self, ctx): + from gns3server.agent.mcp.symbols import get_symbol_handler + from gns3server.services import access_ticket_service + result = get_symbol_handler({"symbol_id": "router.svg"}, ctx) + assert "?token=gns3t_" in result["download_url"] + assert "Bearer" not in result["curl_command"] + ticket = result["download_url"].split("token=")[1] + redeemed = access_ticket_service.redeem_for_path(ticket, "/v3/symbols/router.svg/raw") + assert redeemed is not None and redeemed.username == "admin" + + def test_get_symbol_without_username_omits_token(self, ctx): + from gns3server.agent.mcp.symbols import get_symbol_handler + unauthenticated_ctx = {k: v for k, v in ctx.items() if k != "jwt_username"} + result = get_symbol_handler({"symbol_id": "router.svg"}, unauthenticated_ctx) + assert "token=" not in result["download_url"] + assert "curl_command" not in result + # ── Appliance ─────────────────────────────────────────────────────────── diff --git a/tests/api/routes/controller/test_console_tickets.py b/tests/api/routes/controller/test_access_tickets.py similarity index 61% rename from tests/api/routes/controller/test_console_tickets.py rename to tests/api/routes/controller/test_access_tickets.py index 4518d26d8..fd5f451a6 100644 --- a/tests/api/routes/controller/test_console_tickets.py +++ b/tests/api/routes/controller/test_access_tickets.py @@ -20,7 +20,7 @@ from typing import List import aiohttp import pytest -from fastapi import FastAPI +from fastapi import FastAPI, status from httpx import AsyncClient from httpx_ws import aconnect_ws from httpx_ws.transport import ASGIWebSocketTransport @@ -31,33 +31,33 @@ from gns3server.controller import Controller from gns3server.controller.compute import Compute from gns3server.controller.node import Node from gns3server.controller.project import Project -from gns3server.services.console_tickets import ( - ConsoleTicketService, +from gns3server.services import access_ticket_service +from gns3server.services.access_tickets import ( + AccessTicketService, DEFAULT_TICKET_TTL, TICKET_PREFIX, ) -from gns3server.services import console_ticket_service from gns3server.utils.http_client import HTTPClient from tests.api.routes.controller.test_nodes import FakeComputeConsoleWebSocket -class TestConsoleTicketService: +class TestAccessTicketService: """Unit tests for the in-memory ticket store (fresh instances, no app).""" def test_mint_format(self) -> None: - service = ConsoleTicketService() - ticket = service.mint("admin", 1, "p1", "n1") + service = AccessTicketService() + ticket = service.mint("admin", 1, project_id="p1", node_id="n1") assert ticket.startswith(TICKET_PREFIX) # 12 random bytes -> 16 urlsafe chars; shell-safe alphabet (no quoting hazards) assert len(ticket) == len(TICKET_PREFIX) + 16 assert "." not in ticket # never confusable with a JWT - assert service.mint("admin", 1, "p1", "n1") != ticket + assert service.mint("admin", 1, project_id="p1", node_id="n1") != ticket def test_redeem_valid_ticket_is_multi_use(self) -> None: - service = ConsoleTicketService() - ticket = service.mint("admin", 7, "p1", "n1") + service = AccessTicketService() + ticket = service.mint("admin", 7, project_id="p1", node_id="n1") path_params = {"project_id": "p1", "node_id": "n1"} first = service.redeem(ticket, path_params) second = service.redeem(ticket, path_params) # console clients reconnect, tickets stay usable @@ -67,8 +67,8 @@ class TestConsoleTicketService: def test_redeem_rejects_wrong_binding(self) -> None: - service = ConsoleTicketService() - ticket = service.mint("admin", 0, "p1", "n1") + service = AccessTicketService() + ticket = service.mint("admin", 0, project_id="p1", node_id="n1") assert service.redeem(ticket, {"project_id": "p1", "node_id": "n2"}) is None assert service.redeem(ticket, {"project_id": "p2", "node_id": "n1"}) is None # routes without a node_id path parameter (notifications, wireshark…) never accept a ticket @@ -77,26 +77,54 @@ class TestConsoleTicketService: def test_redeem_rejects_unknown_ticket(self) -> None: - service = ConsoleTicketService() + service = AccessTicketService() assert service.redeem(TICKET_PREFIX + "doesnotexist", {"project_id": "p1", "node_id": "n1"}) is None def test_expired_ticket_is_rejected_and_removed(self) -> None: - service = ConsoleTicketService() - ticket = service.mint("admin", 0, "p1", "n1", ttl=0) + service = AccessTicketService() + ticket = service.mint("admin", 0, project_id="p1", node_id="n1", ttl=0) assert service.redeem(ticket, {"project_id": "p1", "node_id": "n1"}) is None assert ticket not in service._tickets def test_mint_sweeps_expired_entries(self) -> None: - service = ConsoleTicketService() - stale = service.mint("admin", 0, "p1", "n1", ttl=0) - fresh = service.mint("admin", 0, "p1", "n2", ttl=DEFAULT_TICKET_TTL) + service = AccessTicketService() + stale = service.mint("admin", 0, project_id="p1", node_id="n1", ttl=0) + fresh = service.mint("admin", 0, project_id="p1", node_id="n2", ttl=DEFAULT_TICKET_TTL) assert stale not in service._tickets assert fresh in service._tickets + def test_redeem_for_path_valid_ticket_is_multi_use(self) -> None: -class TestConsoleTicketWebSocketAuth: + service = AccessTicketService() + ticket = service.mint("admin", 3, path="/v3/projects/p1/links/l1/capture/file") + first = service.redeem_for_path(ticket, "/v3/projects/p1/links/l1/capture/file") + second = service.redeem_for_path(ticket, "/v3/projects/p1/links/l1/capture/file") + assert first is not None and second is not None + assert first.username == "admin" + assert first.token_version == 3 + + def test_redeem_for_path_rejects_other_path(self) -> None: + + service = AccessTicketService() + ticket = service.mint("admin", 0, path="/v3/projects/p1/links/l1/capture/file") + assert service.redeem_for_path(ticket, "/v3/projects/p1/links/l2/capture/file") is None + assert service.redeem_for_path(ticket, "/v3/projects/p2/links/l1/capture/file") is None + assert service.redeem_for_path(ticket, "/v3/symbols/router.svg/raw") is None + + def test_binding_modes_are_isolated(self) -> None: + + service = AccessTicketService() + # a node-bound ticket must not authenticate REST resources… + ws_ticket = service.mint("admin", 0, project_id="p1", node_id="n1") + assert service.redeem_for_path(ws_ticket, "/v3/projects/p1/nodes/n1/console/ws") is None + # …and a path-bound ticket must not authenticate WebSocket routes + rest_ticket = service.mint("admin", 0, path="/v3/projects/p1/links/l1/capture/file") + assert service.redeem(rest_ticket, {"project_id": "p1", "node_id": "n1"}) is None + + +class TestAccessTicketWebSocketAuth: """ Drive the console WebSocket auth dependency end-to-end through the real routes (the shared service singleton is the one the dependency consults). @@ -158,7 +186,7 @@ class TestConsoleTicketWebSocketAuth: monkeypatch ) -> None: # admin is the seeded superadmin, so the RBAC privilege check is skipped - ticket = console_ticket_service.mint("admin", 0, project.id, node.id) + ticket = access_ticket_service.mint("admin", 0, project_id=project.id, node_id=node.id) self._forward_compute_ws(monkeypatch, [ aiohttp.WSMessage(aiohttp.WSMsgType.TEXT, "device output", None), ]) @@ -181,7 +209,7 @@ class TestConsoleTicketWebSocketAuth: ) -> None: other_node = "00000000-0000-0000-0000-000000000000" - ticket = console_ticket_service.mint("admin", 0, project.id, other_node) + ticket = access_ticket_service.mint("admin", 0, project_id=project.id, node_id=other_node) async with self._ws_client(app, base_client) as client: async with aconnect_ws( f"/v3/projects/{project.id}/nodes/{node.id}/console/ws", @@ -203,7 +231,7 @@ class TestConsoleTicketWebSocketAuth: node: Node ) -> None: # logging out bumps the user's token_version: outstanding tickets must die with it - ticket = console_ticket_service.mint("admin", 999, project.id, node.id) + ticket = access_ticket_service.mint("admin", 999, project_id=project.id, node_id=node.id) async with self._ws_client(app, base_client) as client: async with aconnect_ws( f"/v3/projects/{project.id}/nodes/{node.id}/console/ws", @@ -220,8 +248,59 @@ class TestConsoleTicketWebSocketAuth: ) -> None: # the controller notification stream shares the WS auth dependency but has # no node binding: a console ticket must not authenticate it - ticket = console_ticket_service.mint("admin", 0, "p1", "n1") + ticket = access_ticket_service.mint("admin", 0, project_id="p1", node_id="n1") async with self._ws_client(app, base_client) as client: async with aconnect_ws("/v3/notifications/ws", client, params={"token": ticket}) as ws: notification = await ws.receive_json() assert "Invalid or expired console ticket" in notification["event"]["message"] + + +class TestAccessTicketRestAuth: + """ + Drive the REST auth dependency (get_user_from_token) end-to-end: a + path-bound ticket authenticates exactly one resource path. + """ + + pytestmark = pytest.mark.asyncio + + async def test_rest_accepts_path_bound_ticket( + self, + app: FastAPI, + base_client: AsyncClient + ) -> None: + # base_client carries no Authorization header, so the ?token= parameter is used + ticket = access_ticket_service.mint("admin", 0, path="/v3/projects") + response = await base_client.get("/v3/projects", params={"token": ticket}) + assert response.status_code == status.HTTP_200_OK + + async def test_rest_rejects_ticket_bound_to_other_path( + self, + app: FastAPI, + base_client: AsyncClient + ) -> None: + + ticket = access_ticket_service.mint("admin", 0, path="/v3/symbols/router.svg/raw") + response = await base_client.get("/v3/projects", params={"token": ticket}) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "Invalid or expired access ticket" in response.text + + async def test_rest_rejects_node_bound_ticket( + self, + app: FastAPI, + base_client: AsyncClient + ) -> None: + # node-bound (console) tickets must not authenticate REST resources + ticket = access_ticket_service.mint("admin", 0, project_id="p1", node_id="n1") + response = await base_client.get("/v3/projects", params={"token": ticket}) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + async def test_rest_rejects_ticket_with_stale_token_version( + self, + app: FastAPI, + base_client: AsyncClient + ) -> None: + # logging out bumps the user's token_version: outstanding tickets must die with it + ticket = access_ticket_service.mint("admin", 999, path="/v3/projects") + response = await base_client.get("/v3/projects", params={"token": ticket}) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "revoked" in response.text From f95d4c7a4e4637ea4364041da03a86e786cb4a88 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 29 Aug 2026 01:13:33 +0800 Subject: [PATCH 3/4] fix: update link_capture_download and symbol_get tool descriptions for tickets The tool descriptions still advertised the removed "short-lived JWT" and gave no guidance against URL reconstruction. Align them with node_console's wording: run the returned curl_command verbatim, never rebuild the URL or copy the ticket by hand, re-call the tool once the 10-minute ticket has expired. --- gns3server/agent/mcp/__init__.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/gns3server/agent/mcp/__init__.py b/gns3server/agent/mcp/__init__.py index 1caaa85fe..d6179d24b 100644 --- a/gns3server/agent/mcp/__init__.py +++ b/gns3server/agent/mcp/__init__.py @@ -1009,7 +1009,18 @@ async def link_capture_download( link_id: Annotated[str | None, Field(description="Link UUID (single mode)")] = None, link_ids: Annotated[list[str] | None, Field(description="Batch mode: [\"uuid1\",\"uuid2\"] — get download URLs for multiple captures")] = None, ) -> list[dict[str, Any]]: - """Get download URL(s) for PCAP capture file(s). The URL includes a short-lived JWT (10 min). Use curl to download.""" + """Get download command(s) for PCAP capture file(s). + + Returns a ready-to-run curl command per link with a short-lived access + ticket (10 min) already embedded in the URL. + + IMPORTANT — copy the returned values EXACTLY: + - Run each returned "curl_command" verbatim; it already contains the + full URL with ticket. NEVER construct or edit the URL yourself, and + NEVER copy the ticket by hand — a mistyped ticket is rejected. + - The ticket expires after 10 minutes: call this tool again to get a + fresh one; do not reuse an old URL. + """ params = {"project_id": project_id} if link_ids: params["link_ids"] = link_ids @@ -1299,7 +1310,13 @@ async def server_statistics() -> list[dict[str, Any]]: # async def symbol_get( # symbol_id: Annotated[str, Field(description="Symbol ID (e.g. ':/symbols/router.svg')")], # ) -> list[dict[str, Any]]: -# """Get a download URL for a symbol file (SVG). The URL includes a short-lived JWT (10 min). Use curl to download.""" +# """Get a download URL for a symbol file (SVG). +# +# Returns a ready-to-run curl command with a short-lived access ticket +# (10 min) embedded in the URL. Run the returned "curl_command" verbatim — +# never reconstruct the URL or copy the ticket by hand. Re-call this tool +# once the ticket has expired. +# """ # return await asyncio.to_thread(_run_handler_sync, get_symbol_handler, { # "symbol_id": symbol_id, # }) From 3cf9dbff5b19964427c0110acf3c00ba65eeaa87 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 29 Aug 2026 10:09:05 +0800 Subject: [PATCH 4/4] fix: handle client disconnect in compute console WebSocket forwarding The compute-side telnet_forward (and vnc_forward) let the WebSocketDisconnect that starlette raises from send_bytes once the peer is gone escape to the generic task-exception handler. Its str() is empty, so every client disconnect during active node output logged a message-less warning: WARNING gns3server.compute.base_node:658 Exception while forwarding WebSocket data to TELNET server: Catch WebSocketDisconnect in both forwarders and log it at INFO as the normal end of a session, and format the remaining task exceptions with !r so their type stays visible. Mirrors the controller-side fix for the same scenario (2324dcd74). --- gns3server/compute/base_node.py | 32 ++++++++++++++----- tests/compute/test_base_node.py | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/gns3server/compute/base_node.py b/gns3server/compute/base_node.py index f22fa97c0..65b816e5c 100644 --- a/gns3server/compute/base_node.py +++ b/gns3server/compute/base_node.py @@ -640,10 +640,20 @@ class BaseNode: async def telnet_forward(telnet_reader): - while not telnet_reader.at_eof(): - data = await telnet_reader.read(1024) - if data: - await websocket.send_bytes(data) + try: + while not telnet_reader.at_eof(): + data = await telnet_reader.read(1024) + if data: + await websocket.send_bytes(data) + except WebSocketDisconnect: + # the client disconnected while node output was still streaming: + # normal end of the session, not an error. Starlette raises + # WebSocketDisconnect (whose str() is empty) from send once the + # peer is gone, which used to surface as a message-less warning. + log.info( + f"Client {websocket.client.host}:{websocket.client.port} has disconnected from compute" + f" console WebSocket while node output was being forwarded" + ) # keep forwarding websocket data in both direction if sys.version_info >= (3, 11, 0): @@ -657,7 +667,7 @@ class BaseNode: if task.exception(): log.warning( f"Exception while forwarding WebSocket data to " - f"{self._console_type.upper()} server: {task.exception()}" + f"{self._console_type.upper()} server: {task.exception()!r}" ) for task in pending: task.cancel() @@ -728,8 +738,16 @@ class BaseNode: data = await vnc_reader.read(65536) # Larger buffer for VNC frames if data: await websocket.send_bytes(data) + except WebSocketDisconnect: + # the browser disconnected while VNC frames were still streaming + # (starlette raises WebSocketDisconnect with an empty str() from + # send once the peer is gone — not an error) + log.info( + f"Client {websocket.client.host}:{websocket.client.port} has disconnected from compute " + f"VNC console WebSocket while frames were being forwarded" + ) except Exception as e: - log.warning(f"Exception while forwarding VNC data to WebSocket: {e}") + log.warning(f"Exception while forwarding VNC data to WebSocket: {e!r}") # Keep forwarding WebSocket data in both directions if sys.version_info >= (3, 11, 0): @@ -741,7 +759,7 @@ class BaseNode: done, pending = await asyncio.wait(aws, return_when=asyncio.FIRST_COMPLETED) for task in done: if task.exception(): - log.warning(f"Exception while forwarding WebSocket data to VNC server: {task.exception()}") + log.warning(f"Exception while forwarding WebSocket data to VNC server: {task.exception()!r}") for task in pending: task.cancel() diff --git a/tests/compute/test_base_node.py b/tests/compute/test_base_node.py index 9aa97cf81..7eca945e1 100644 --- a/tests/compute/test_base_node.py +++ b/tests/compute/test_base_node.py @@ -453,3 +453,57 @@ async def test_stop_ubridge_clears_marker_bridges(compute_project, manager): node._marker_filter_bridges["m", "L1"] = "VPCS-10" await node._stop_ubridge() assert node._marker_filter_bridges == {} + + +class _GoneConsoleWebsocket: + """ + Stand-in for the compute-side WebSocket when the peer (the controller, or a + browser) is already gone: receive() yields the disconnect message and + send_bytes raises WebSocketDisconnect — what starlette raises from send + after the transport reports OSError. + """ + + def __init__(self): + from types import SimpleNamespace + self.client = SimpleNamespace(host="127.0.0.1", port=5000) + + async def receive(self): + return {"type": "websocket.disconnect"} + + async def send_bytes(self, data): + from starlette.websockets import WebSocketDisconnect + raise WebSocketDisconnect(code=1006) + + +@pytest.mark.asyncio +async def test_console_websocket_client_disconnect_while_node_output_streams( + compute_project, manager, port_manager, monkeypatch, caplog): + # regression test: the client disconnects while the node is still streaming + # console output. telnet_forward used to let the (empty-str) WebSocketDisconnect + # from send_bytes escape, logging a message-less WARNING. + import asyncio + import logging + + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + node.status = "started" + + telnet_reader = MagicMock() + telnet_reader.at_eof.return_value = False + telnet_reader.read = AsyncioMagicMock(return_value=b"device output") + telnet_writer = MagicMock() + telnet_writer.wait_closed = AsyncioMagicMock() + + async def fake_open_connection(*args, **kwargs): + return telnet_reader, telnet_writer + + monkeypatch.setattr(asyncio, "open_connection", fake_open_connection) + + with caplog.at_level(logging.INFO): + await node.start_websocket_console(_GoneConsoleWebsocket()) + + assert telnet_writer.close.called + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + assert any( + "has disconnected from compute console WebSocket while node output" in r.message + for r in caplog.records + )