mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-11-03 14:01:15 +02:00
fix: replace Bearer JWTs with path-bound access tickets in MCP download tools
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=<ticket> and the curl commands no longer carry a Bearer header.
This commit is contained in:
8 files changed
+293
-88
No files matched your search
@@ -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 ───────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
+102
-23
@@ -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
|
||||
Reference in new issue
Block a user