Skip to content

Commit cc87f5b

Browse files
committed
addressed review comments
1 parent 607063d commit cc87f5b

7 files changed

Lines changed: 114 additions & 91 deletions

File tree

README.md

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -302,20 +302,20 @@ MCP (Model Context Protocol) servers provide tools and capabilities to the AI ag
302302

303303
**Basic Configuration Structure:**
304304

305-
Each MCP server requires three fields:
305+
Each MCP server requires two fields:
306306
- `name`: Unique identifier for the MCP server
307-
- `provider_id`: MCP provider identification (typically `"model-context-protocol"`)
308307
- `url`: The endpoint where the MCP server is running
309308

309+
And one optional field:
310+
- `provider_id`: MCP provider identification (defaults to `"model-context-protocol"`)
311+
310312
**Minimal Example:**
311313

312314
```yaml
313315
mcp_servers:
314316
- name: "filesystem-tools"
315-
provider_id: "model-context-protocol"
316317
url: "http://localhost:3000"
317318
- name: "git-tools"
318-
provider_id: "model-context-protocol"
319319
url: "http://localhost:3001"
320320
```
321321

@@ -332,7 +332,6 @@ Store authentication tokens in secret files and reference them in your configura
332332
```yaml
333333
mcp_servers:
334334
- name: "api-service"
335-
provider_id: "model-context-protocol"
336335
url: "http://api-service:8080"
337336
authorization_headers:
338337
Authorization: "/var/secrets/api-token" # Path to file containing token
@@ -356,7 +355,6 @@ Use the special `"kubernetes"` keyword to automatically use the authenticated us
356355
```yaml
357356
mcp_servers:
358357
- name: "k8s-internal-service"
359-
provider_id: "model-context-protocol"
360358
url: "http://internal-mcp.default.svc.cluster.local:8080"
361359
authorization_headers:
362360
Authorization: "kubernetes" # Uses user's k8s token from request auth
@@ -371,7 +369,6 @@ Use the special `"client"` keyword to allow clients to provide custom tokens per
371369
```yaml
372370
mcp_servers:
373371
- name: "user-specific-service"
374-
provider_id: "model-context-protocol"
375372
url: "http://user-service:8080"
376373
authorization_headers:
377374
Authorization: "client" # Token provided via MCP-HEADERS
@@ -387,7 +384,9 @@ curl -X POST "http://localhost:8080/v1/query" \
387384
-d '{"query": "Get my data"}'
388385
```
389386

390-
**Note**: The `MCP-HEADERS` dictionary is keyed by **server name** (not URL), matching the `name` field in your MCP server configuration.
387+
**Note**: `MCP-HEADERS` is an **HTTP request header** containing a JSON-encoded dictionary. The dictionary is keyed by **server name** (not URL), matching the `name` field in your MCP server configuration. Each server name maps to another dictionary containing the HTTP headers to forward to that specific MCP server.
388+
389+
**Structure**: `MCP-HEADERS: {"<server-name>": {"<header-name>": "<header-value>", ...}, ...}`
391390

392391
##### Combining Authentication Methods
393392

@@ -397,21 +396,18 @@ You can mix and match authentication methods across different MCP servers, and e
397396
mcp_servers:
398397
# Static credentials for public API
399398
- name: "weather-api"
400-
provider_id: "model-context-protocol"
401399
url: "http://weather-api:8080"
402400
authorization_headers:
403401
X-API-Key: "/var/secrets/weather-api-key"
404402
405403
# Kubernetes auth for internal services
406404
- name: "internal-db"
407-
provider_id: "model-context-protocol"
408405
url: "http://db-mcp.cluster.local:8080"
409406
authorization_headers:
410407
Authorization: "kubernetes"
411408
412409
# Mixed: static API key + per-user token
413410
- name: "multi-tenant-service"
414-
provider_id: "model-context-protocol"
415411
url: "http://multi-tenant:8080"
416412
authorization_headers:
417413
X-Service-Key: "/var/secrets/service-key" # Static service credential

dev-tools/MANUAL_TESTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ The mock server should return unique tool names for each auth type:
9191
- `mock_tool_client` - from `mock-client-auth`
9292

9393
Check the Lightspeed Core logs, you should see:
94-
```
94+
```text
9595
DEBUG Configured 3 MCP tools: ['mock-file-auth', 'mock-k8s-auth', 'mock-client-auth']
9696
```
9797

dev-tools/mcp-mock-server/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ This mock server helps developers:
1111
- Develop and test MCP-related features
1212
- Test both HTTP and HTTPS connections
1313

14+
**⚠️ Testing Only:** This server is single-threaded and handles requests sequentially. It is designed purely for development and testing purposes, not for production or high-load scenarios.
15+
1416
## Features
1517

1618
-**Pure Python** - No external dependencies (uses stdlib only)

dev-tools/mcp-mock-server/server.py

Lines changed: 64 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,10 @@
2323
from http.server import HTTPServer, BaseHTTPRequestHandler
2424
from datetime import datetime
2525
from pathlib import Path
26-
from typing import Dict
2726

2827

2928
# Global storage for captured headers (last request)
30-
last_headers: Dict[str, str] = {}
29+
last_headers: dict[str, str] = {}
3130
request_log: list = []
3231

3332

@@ -79,19 +78,22 @@ def do_POST(self) -> None: # pylint: disable=invalid-name
7978

8079
# Determine tool name based on authorization header to avoid collisions
8180
auth_header = self.headers.get("Authorization", "")
82-
if "test-secret-token" in auth_header:
83-
tool_name = "mock_tool_file"
84-
tool_desc = "Mock tool with file-based auth"
85-
elif "my-k8s-token" in auth_header:
86-
tool_name = "mock_tool_k8s"
87-
tool_desc = "Mock tool with Kubernetes token"
88-
elif "my-client-token" in auth_header:
89-
tool_name = "mock_tool_client"
90-
tool_desc = "Mock tool with client-provided token"
91-
else:
92-
# No auth header or unrecognized token
93-
tool_name = "mock_tool_no_auth"
94-
tool_desc = "Mock tool with no authorization"
81+
82+
# Match based on token content
83+
match auth_header:
84+
case _ if "test-secret-token" in auth_header:
85+
tool_name = "mock_tool_file"
86+
tool_desc = "Mock tool with file-based auth"
87+
case _ if "my-k8s-token" in auth_header:
88+
tool_name = "mock_tool_k8s"
89+
tool_desc = "Mock tool with Kubernetes token"
90+
case _ if "my-client-token" in auth_header:
91+
tool_name = "mock_tool_client"
92+
tool_desc = "Mock tool with client-provided token"
93+
case _:
94+
# No auth header or unrecognized token
95+
tool_name = "mock_tool_no_auth"
96+
tool_desc = "Mock tool with no authorization"
9597

9698
# Handle MCP protocol methods
9799
if method == "initialize":
@@ -150,51 +152,53 @@ def do_POST(self) -> None: # pylint: disable=invalid-name
150152

151153
def do_GET(self) -> None: # pylint: disable=invalid-name
152154
"""Handle GET requests (debug endpoints)."""
153-
# Debug endpoint to view captured headers
154-
if self.path == "/debug/headers":
155-
self.send_response(200)
156-
self.send_header("Content-Type", "application/json")
157-
self.end_headers()
158-
response = {
159-
"last_headers": last_headers,
160-
"request_count": len(request_log),
161-
}
162-
self.wfile.write(json.dumps(response, indent=2).encode())
163-
164-
# Debug endpoint to view request log
165-
elif self.path == "/debug/requests":
166-
self.send_response(200)
167-
self.send_header("Content-Type", "application/json")
168-
self.end_headers()
169-
self.wfile.write(json.dumps(request_log, indent=2).encode())
170-
171-
# Root endpoint - show help
172-
elif self.path == "/":
173-
self.send_response(200)
174-
self.send_header("Content-Type", "text/html")
175-
self.end_headers()
176-
help_html = """
177-
<html>
178-
<head><title>MCP Mock Server</title></head>
179-
<body>
180-
<h1>MCP Mock Server</h1>
181-
<p>This is a development mock server for testing MCP integrations.</p>
182-
<h2>Debug Endpoints:</h2>
183-
<ul>
184-
<li><a href="/debug/headers">/debug/headers</a> - View last captured headers</li>
185-
<li><a href="/debug/requests">/debug/requests</a> - View recent request log</li>
186-
</ul>
187-
<h2>MCP Endpoints:</h2>
188-
<ul>
189-
<li>POST /mcp/v1/list_tools - Mock MCP tools endpoint</li>
190-
</ul>
191-
</body>
192-
</html>
193-
"""
194-
self.wfile.write(help_html.encode())
195-
else:
196-
self.send_response(404)
197-
self.end_headers()
155+
# Handle different GET endpoints
156+
match self.path:
157+
case "/debug/headers":
158+
self._send_json_response(
159+
{"last_headers": last_headers, "request_count": len(request_log)}
160+
)
161+
case "/debug/requests":
162+
self._send_json_response(request_log)
163+
case "/":
164+
self._send_help_page()
165+
case _:
166+
self.send_response(404)
167+
self.end_headers()
168+
169+
def _send_json_response(self, data: dict | list) -> None:
170+
"""Send a JSON response."""
171+
self.send_response(200)
172+
self.send_header("Content-Type", "application/json")
173+
self.end_headers()
174+
self.wfile.write(json.dumps(data, indent=2).encode())
175+
176+
def _send_help_page(self) -> None:
177+
"""Send HTML help page for root endpoint."""
178+
self.send_response(200)
179+
self.send_header("Content-Type", "text/html")
180+
self.end_headers()
181+
help_html = """<!DOCTYPE html>
182+
<html>
183+
<head><title>MCP Mock Server</title></head>
184+
<body>
185+
<h1>MCP Mock Server</h1>
186+
<p>Development mock server for testing MCP integrations.</p>
187+
<h2>Debug Endpoints:</h2>
188+
<ul>
189+
<li><a href="/debug/headers">/debug/headers</a> - View captured headers</li>
190+
<li><a href="/debug/requests">/debug/requests</a> - View request log</li>
191+
</ul>
192+
<h2>MCP Protocol:</h2>
193+
<p>POST requests to <code>/</code> with JSON-RPC format:</p>
194+
<ul>
195+
<li><code>{"jsonrpc": "2.0", "id": 1, "method": "initialize"}</code></li>
196+
<li><code>{"jsonrpc": "2.0", "id": 1, "method": "tools/list"}</code></li>
197+
</ul>
198+
</body>
199+
</html>
200+
"""
201+
self.wfile.write(help_html.encode())
198202

199203

200204
def generate_self_signed_cert(cert_dir: Path) -> tuple[Path, Path]:

dev-tools/mcp-mock-server/test_mock_mcp_server.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,34 +103,38 @@ def make_request(
103103
def test_http_mcp_list_tools(mock_server):
104104
"""Test the MCP list_tools endpoint over HTTP."""
105105
status, response = make_request(
106-
f"{mock_server['http_url']}/mcp/v1/list_tools",
106+
f"{mock_server['http_url']}/",
107107
method="POST",
108-
data={"test": "data"},
108+
data={"jsonrpc": "2.0", "id": 1, "method": "tools/list"},
109109
headers={"Authorization": "Bearer test-token-123"},
110110
)
111111

112112
assert status == 200, f"Expected 200 OK, got {status}"
113113
assert isinstance(response, dict), f"Expected dict, got {type(response)}"
114-
assert "tools" in response, "Response should contain 'tools' key"
115-
assert isinstance(response["tools"], list), "Tools should be a list"
116-
assert len(response["tools"]) == 1, "Should have 1 mock tool"
117-
assert (
118-
response["tools"][0]["name"] == "mock_tool"
119-
), "Tool name should be 'mock_tool'"
114+
assert "result" in response, "Response should contain 'result' key (JSON-RPC)"
115+
assert "tools" in response["result"], "Result should contain 'tools' key"
116+
assert isinstance(response["result"]["tools"], list), "Tools should be a list"
117+
assert len(response["result"]["tools"]) == 1, "Should have 1 mock tool"
118+
# Tool name varies based on auth header
119+
assert response["result"]["tools"][0]["name"].startswith(
120+
"mock_tool"
121+
), "Tool name should start with 'mock_tool'"
120122

121123

122124
def test_https_mcp_list_tools(mock_server):
123125
"""Test the MCP list_tools endpoint over HTTPS."""
124126
status, response = make_request(
125-
f"{mock_server['https_url']}/mcp/v1/list_tools",
127+
f"{mock_server['https_url']}/",
126128
method="POST",
127-
data={"test": "data"},
129+
data={"jsonrpc": "2.0", "id": 1, "method": "tools/list"},
128130
headers={"Authorization": "Bearer test-https-token"},
129131
use_https=True,
130132
)
131133

132134
assert status == 200, f"Expected 200 OK over HTTPS, got {status}"
133135
assert isinstance(response, dict), f"Expected dict, got {type(response)}"
136+
assert "result" in response, "Response should contain 'result' key (JSON-RPC)"
137+
assert "tools" in response["result"], "Result should contain 'tools' key"
134138

135139

136140
def test_debug_headers_endpoint(mock_server):

src/utils/mcp_auth_headers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ def resolve_authorization_headers(
1414
1515
Parameters:
1616
authorization_headers: Map of header names to secret locations or special keywords.
17-
- If value is "kubernetes": live is unchanged. We substitute it during request.
18-
- If value is "client": live it unchanged. . We substitute it during request.
17+
- If value is "kubernetes": leave is unchanged. We substitute it during request.
18+
- If value is "client": leave it unchanged. . We substitute it during request.
1919
- Otherwise: Treat as file path and read the secret from that file
2020
2121
Returns:

tests/unit/models/config/test_model_context_protocol_server.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
# pyright: reportCallIssue=false
44

5+
from pathlib import Path
6+
57
import pytest
68

79
from pydantic import ValidationError
@@ -137,20 +139,31 @@ def test_configuration_multiple_mcp_servers() -> None:
137139
assert cfg.mcp_servers[2].name == "server3"
138140

139141

140-
def test_model_context_protocol_server_with_authorization_headers() -> None:
142+
def test_model_context_protocol_server_with_authorization_headers(
143+
tmp_path: Path,
144+
) -> None:
141145
"""Test ModelContextProtocolServer with authorization headers."""
146+
auth_file = tmp_path / "auth.txt"
147+
auth_file.write_text("my-secret")
148+
api_key_file = tmp_path / "api_key.txt"
149+
api_key_file.write_text("api-key-secret")
150+
142151
mcp = ModelContextProtocolServer(
143152
name="auth-server",
144153
url="http://localhost:8080",
145154
authorization_headers={
146-
"Authorization": "my-secret",
147-
"X-API-Key": "api-key-secret",
155+
"Authorization": str(auth_file),
156+
"X-API-Key": str(api_key_file),
148157
},
149158
)
150159
assert mcp is not None
151160
assert mcp.name == "auth-server"
152161
assert mcp.url == "http://localhost:8080"
153162
assert mcp.authorization_headers == {
163+
"Authorization": str(auth_file),
164+
"X-API-Key": str(api_key_file),
165+
}
166+
assert mcp.resolved_authorization_headers == {
154167
"Authorization": "my-secret",
155168
"X-API-Key": "api-key-secret",
156169
}
@@ -178,19 +191,22 @@ def test_model_context_protocol_server_client_special_case() -> None:
178191
assert mcp.authorization_headers == {"Authorization": "client"}
179192

180193

181-
def test_configuration_mcp_servers_with_mixed_auth_headers() -> None:
194+
def test_configuration_mcp_servers_with_mixed_auth_headers(tmp_path: Path) -> None:
182195
"""
183196
Test Configuration with MCP servers having mixed authorization headers.
184197
185198
Verifies backward compatibility (servers without auth headers) and
186199
new functionality (servers with auth headers) work together.
187200
"""
201+
secret_file = tmp_path / "secret.txt"
202+
secret_file.write_text("my-secret")
203+
188204
mcp_servers = [
189205
ModelContextProtocolServer(name="server-no-auth", url="http://localhost:8080"),
190206
ModelContextProtocolServer(
191207
name="server-with-secret",
192208
url="http://localhost:8081",
193-
authorization_headers={"Authorization": "my-secret"},
209+
authorization_headers={"Authorization": str(secret_file)},
194210
),
195211
ModelContextProtocolServer(
196212
name="server-with-k8s",
@@ -225,7 +241,8 @@ def test_configuration_mcp_servers_with_mixed_auth_headers() -> None:
225241

226242
# Server with secret reference
227243
assert cfg.mcp_servers[1].name == "server-with-secret"
228-
assert cfg.mcp_servers[1].authorization_headers == {"Authorization": "my-secret"}
244+
assert cfg.mcp_servers[1].authorization_headers == {"Authorization": str(secret_file)}
245+
assert cfg.mcp_servers[1].resolved_authorization_headers == {"Authorization": "my-secret"}
229246

230247
# Server with kubernetes special case
231248
assert cfg.mcp_servers[2].name == "server-with-k8s"
@@ -279,4 +296,4 @@ def test_model_context_protocol_server_resolved_headers_empty() -> None:
279296
url="http://localhost:8080",
280297
)
281298
assert mcp is not None
282-
assert mcp.resolved_authorization_headers == {}
299+
assert not mcp.resolved_authorization_headers

0 commit comments

Comments
 (0)