Skip to content

Commit 1f9d503

Browse files
author
Olivier Gintrand
committed
refactor: code review cleanup — remove dead code, factor helpers, fix tests
- Remove duplicate meta-server sections in CREATE and EDIT modals (admin.html) - Remove dead imports: ToolEmbedding, top-level or_ (service.py) - Fix docstring: meta-server exposes 12 tools, not 6 (service.py) - Factor _resolve_effective_email() helper for JWT extraction (service.py) - Factor _apply_scope_header() helper for X-Scope processing (meta_router.py) - Move inline 'import json' to module level (meta_router.py) - Fix test assertion: len(defs) == 12, add new meta-tool assertions (test_meta_server.py) Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com>
1 parent 85a3a83 commit 1f9d503

4 files changed

Lines changed: 49 additions & 143 deletions

File tree

mcpgateway/meta_server/service.py

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,8 @@
3535
import time
3636
from typing import Any, Dict, List, Optional, Set
3737

38-
# Third-Party
39-
from sqlalchemy import or_
40-
4138
# First-Party
42-
from mcpgateway.db import fresh_db_session, get_db, Tool, ToolEmbedding
39+
from mcpgateway.db import fresh_db_session, get_db, Tool
4340
from mcpgateway.meta_server.schemas import (
4441
AuthorizeAllGatewaysResponse,
4542
AuthorizeGatewayResponse,
@@ -84,7 +81,7 @@ class MetaServerService:
8481
>>> defs = service.get_meta_tool_definitions()
8582
>>> isinstance(defs, list)
8683
True
87-
>>> len(defs) == 6
84+
>>> len(defs) == 12
8885
True
8986
"""
9087

@@ -96,12 +93,6 @@ def get_meta_tool_definitions(self) -> List[Dict[str, Any]]:
9693
9794
Returns:
9895
List of meta-tool definition dicts with keys: name, description, inputSchema.
99-
100-
Examples:
101-
>>> service = MetaServerService()
102-
>>> tools = service.get_meta_tool_definitions()
103-
>>> {t["name"] for t in tools} == {"search_tools", "list_tools", "describe_tool", "execute_tool", "get_tool_categories", "get_similar_tools"}
104-
True
10596
"""
10697
return [{"name": name, "description": defn["description"], "inputSchema": defn["input_schema"]} for name, defn in META_TOOL_DEFINITIONS.items()]
10798

@@ -282,6 +273,34 @@ def _extract_user_context(kwargs: Dict[str, Any]) -> tuple:
282273
kwargs.get("request_headers"),
283274
)
284275

276+
@staticmethod
277+
def _resolve_effective_email(
278+
user_email: Optional[str],
279+
request_headers: Optional[Dict[str, str]],
280+
) -> Optional[str]:
281+
"""Resolve effective user email from explicit param or JWT in Authorization header.
282+
283+
Prefers the explicit ``user_email`` parameter. Falls back to extracting
284+
the email claim from the Bearer JWT in the Authorization header.
285+
The JWT signature is NOT verified here — upstream middleware is responsible
286+
for authentication. This is only used for user identity resolution.
287+
"""
288+
if user_email:
289+
return user_email.strip().lower() if isinstance(user_email, str) else None
290+
if not request_headers:
291+
return None
292+
auth_header = request_headers.get("authorization", "")
293+
if not auth_header.startswith("Bearer "):
294+
return None
295+
try:
296+
import jwt as pyjwt # pylint: disable=import-outside-toplevel
297+
token = auth_header[7:]
298+
payload = pyjwt.decode(token, options={"verify_signature": False})
299+
email = payload.get("email") or payload.get("sub")
300+
return email.strip().lower() if email else None
301+
except Exception:
302+
return None
303+
285304
# ------------------------------------------------------------------
286305
# Implemented handlers
287306
# ------------------------------------------------------------------
@@ -1085,19 +1104,7 @@ async def _authorize_gateway(
10851104
from mcpgateway.services.token_storage_service import TokenStorageService
10861105

10871106
# Resolve user_email: prefer explicit param, fall back to JWT in request_headers
1088-
effective_email = user_email
1089-
if not effective_email and request_headers:
1090-
auth_header = request_headers.get("authorization", "")
1091-
if auth_header.startswith("Bearer "):
1092-
try:
1093-
import jwt as pyjwt # pylint: disable=import-outside-toplevel
1094-
token = auth_header[7:]
1095-
payload = pyjwt.decode(token, options={"verify_signature": False})
1096-
effective_email = payload.get("email") or payload.get("sub")
1097-
if effective_email:
1098-
effective_email = effective_email.strip().lower()
1099-
except Exception:
1100-
pass
1107+
effective_email = self._resolve_effective_email(user_email, request_headers)
11011108

11021109
gateway_name = arguments.get("gateway_name", "")
11031110
if not gateway_name:
@@ -1210,19 +1217,7 @@ async def _authorize_all_gateways(
12101217
from mcpgateway.services.token_storage_service import TokenStorageService
12111218

12121219
# Resolve user_email from JWT if not provided
1213-
effective_email = user_email
1214-
if not effective_email and request_headers:
1215-
auth_header = request_headers.get("authorization", "")
1216-
if auth_header.startswith("Bearer "):
1217-
try:
1218-
import jwt as pyjwt # pylint: disable=import-outside-toplevel
1219-
token = auth_header[7:]
1220-
payload = pyjwt.decode(token, options={"verify_signature": False})
1221-
effective_email = payload.get("email") or payload.get("sub")
1222-
if effective_email:
1223-
effective_email = effective_email.strip().lower()
1224-
except Exception:
1225-
pass
1220+
effective_email = self._resolve_effective_email(user_email, request_headers)
12261221

12271222
try:
12281223
db_gen = get_db()

mcpgateway/routers/meta_router.py

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"""
1717

1818
# Standard
19+
import json
1920
import time
2021
from typing import Any, Dict, Optional
2122

@@ -60,6 +61,16 @@
6061
router = APIRouter(prefix="/meta", tags=["Meta Tools"])
6162

6263

64+
def _apply_scope_header(arguments: Dict[str, Any], x_scope: Optional[str]) -> Dict[str, Any]:
65+
"""Apply X-Scope header to arguments dict, parsing JSON if provided."""
66+
if x_scope:
67+
try:
68+
arguments["scope"] = json.loads(x_scope)
69+
except (json.JSONDecodeError, ValueError):
70+
logger.warning(f"Invalid X-Scope header: {x_scope}")
71+
return arguments
72+
73+
6374
@router.post("/describe_tool", response_model=DescribeToolResponse)
6475
async def describe_tool(
6576
req: DescribeToolRequest,
@@ -198,13 +209,7 @@ async def search_tools(
198209
meta_service = get_meta_server_service()
199210

200211
# Build arguments dict with scope from header if provided
201-
arguments = req.model_dump()
202-
if x_scope:
203-
try:
204-
import json
205-
arguments["scope"] = json.loads(x_scope)
206-
except json.JSONDecodeError:
207-
logger.warning(f"Invalid X-Scope header: {x_scope}")
212+
arguments = _apply_scope_header(req.model_dump(), x_scope)
208213

209214
# Call the service handler
210215
result = await meta_service._search_tools(arguments)
@@ -241,13 +246,7 @@ async def list_tools(
241246
meta_service = get_meta_server_service()
242247

243248
# Build arguments dict with scope from header if provided
244-
arguments = req.model_dump()
245-
if x_scope:
246-
try:
247-
import json
248-
arguments["scope"] = json.loads(x_scope)
249-
except json.JSONDecodeError:
250-
logger.warning(f"Invalid X-Scope header: {x_scope}")
249+
arguments = _apply_scope_header(req.model_dump(), x_scope)
251250

252251
# Call the service handler
253252
result = await meta_service._list_tools(arguments)
@@ -284,13 +283,7 @@ async def get_similar_tools(
284283
meta_service = get_meta_server_service()
285284

286285
# Build arguments dict with scope from header if provided
287-
arguments = req.model_dump()
288-
if x_scope:
289-
try:
290-
import json
291-
arguments["scope"] = json.loads(x_scope)
292-
except json.JSONDecodeError:
293-
logger.warning(f"Invalid X-Scope header: {x_scope}")
286+
arguments = _apply_scope_header(req.model_dump(), x_scope)
294287

295288
# Call the service handler
296289
result = await meta_service._get_similar_tools(arguments)

mcpgateway/templates/admin.html

Lines changed: 0 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -3478,48 +3478,6 @@ <h3 class="text-lg font-bold mb-4 dark:text-gray-200">
34783478
</div>
34793479
</div>
34803480

3481-
<!-- Meta-Server Configuration Section -->
3482-
<div class="mt-6 border-t border-gray-200 dark:border-gray-700 pt-4">
3483-
<div class="flex items-center mb-4">
3484-
<input
3485-
type="checkbox"
3486-
name="meta_server_enabled"
3487-
id="server-meta-enabled"
3488-
class="form-checkbox h-5 w-5 text-indigo-600 dark:bg-gray-800 dark:border-gray-600"
3489-
onchange="document.getElementById('server-meta-config-section').classList.toggle('hidden', !this.checked)"
3490-
/>
3491-
<label
3492-
for="server-meta-enabled"
3493-
class="ml-2 text-sm font-medium text-gray-700 dark:text-gray-400"
3494-
>Enable as Meta-Server (Tool Router)</label
3495-
>
3496-
</div>
3497-
<p class="text-xs text-gray-500 dark:text-gray-400 mb-3">
3498-
When enabled, this server exposes meta-tools (search, list, describe, execute) instead of individual underlying tools.
3499-
</p>
3500-
3501-
<!-- Meta-Server Configuration Fields (hidden by default) -->
3502-
<div id="server-meta-config-section" class="hidden space-y-4 pl-6 border-l-2 border-indigo-200 dark:border-indigo-800">
3503-
<div class="flex items-center">
3504-
<input
3505-
type="checkbox"
3506-
name="hide_underlying_tools"
3507-
id="server-hide-underlying-tools"
3508-
class="form-checkbox h-5 w-5 text-indigo-600 dark:bg-gray-800 dark:border-gray-600"
3509-
checked
3510-
/>
3511-
<label
3512-
for="server-hide-underlying-tools"
3513-
class="ml-2 text-sm font-medium text-gray-700 dark:text-gray-400"
3514-
>Hide underlying tools from tool listing</label
3515-
>
3516-
<p class="ml-2 text-xs text-gray-500 dark:text-gray-400">
3517-
Clients will only see the meta-tools, not the individual backend tools.
3518-
</p>
3519-
</div>
3520-
</div>
3521-
</div>
3522-
35233481
<div class="mt-6">
35243482
<button
35253483
type="submit"
@@ -11951,48 +11909,6 @@ <h3 class="text-lg font-medium text-gray-900 dark:text-gray-100">
1195111909
</div>
1195211910
</div>
1195311911

11954-
<!-- Meta-Server Configuration Section -->
11955-
<div class="mt-6 border-t border-gray-200 dark:border-gray-700 pt-4">
11956-
<div class="flex items-center mb-4">
11957-
<input
11958-
type="checkbox"
11959-
name="meta_server_enabled"
11960-
id="edit-server-meta-enabled"
11961-
class="form-checkbox h-5 w-5 text-indigo-600 dark:bg-gray-800 dark:border-gray-600"
11962-
onchange="document.getElementById('edit-server-meta-config-section').classList.toggle('hidden', !this.checked)"
11963-
/>
11964-
<label
11965-
for="edit-server-meta-enabled"
11966-
class="ml-2 text-sm font-medium text-gray-700 dark:text-gray-400"
11967-
>Enable as Meta-Server (Tool Router)</label
11968-
>
11969-
</div>
11970-
<p class="text-xs text-gray-500 dark:text-gray-400 mb-3">
11971-
When enabled, this server exposes meta-tools (search, list, describe, execute) instead of individual underlying tools.
11972-
</p>
11973-
11974-
<!-- Meta-Server Configuration Fields (hidden by default) -->
11975-
<div id="edit-server-meta-config-section" class="hidden space-y-4 pl-6 border-l-2 border-indigo-200 dark:border-indigo-800">
11976-
<div class="flex items-center">
11977-
<input
11978-
type="checkbox"
11979-
name="hide_underlying_tools"
11980-
id="edit-server-hide-underlying-tools"
11981-
class="form-checkbox h-5 w-5 text-indigo-600 dark:bg-gray-800 dark:border-gray-600"
11982-
checked
11983-
/>
11984-
<label
11985-
for="edit-server-hide-underlying-tools"
11986-
class="ml-2 text-sm font-medium text-gray-700 dark:text-gray-400"
11987-
>Hide underlying tools from tool listing</label
11988-
>
11989-
<p class="ml-2 text-xs text-gray-500 dark:text-gray-400">
11990-
Clients will only see the meta-tools, not the individual backend tools.
11991-
</p>
11992-
</div>
11993-
</div>
11994-
</div>
11995-
1199611912
<div
1199711913
class="mt-6 flex justify-end space-x-3 bg-white dark:bg-gray-900 dark:text-gray-100"
1199811914
>

tests/unit/mcpgateway/test_meta_server.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,12 @@ def test_get_meta_tool_definitions(self):
350350
"""Test that meta-tool definitions are returned correctly."""
351351
service = MetaServerService()
352352
defs = service.get_meta_tool_definitions()
353-
assert len(defs) == 6
353+
assert len(defs) == 12
354354
names = {d["name"] for d in defs}
355355
assert "search_tools" in names
356356
assert "execute_tool" in names
357+
assert "list_resources" in names
358+
assert "authorize_gateway" in names
357359

358360
def test_is_meta_server(self):
359361
"""Test is_meta_server check."""

0 commit comments

Comments
 (0)