Skip to content

Commit 7e23cc9

Browse files
committed
fix(auth): enforce catalog scope permissions
Restrict gateway ownership transfers to the caller's token teams and require gateways.create for admin catalog registration routes. Add deny-path regression coverage for scoped transfers and insufficient gateway permissions. Signed-off-by: Lang-Akshay <akshay.shinde26@ibm.com>
1 parent 769ab8a commit 7e23cc9

6 files changed

Lines changed: 106 additions & 1 deletion

File tree

mcpgateway/admin.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13138,13 +13138,15 @@ async def transfer_gateway_ownership(
1313813138
Updated GatewayRead with new ownership.
1313913139
"""
1314013140
actor_email = get_user_email(_user)
13141+
token_teams = extract_token_team_ids(_user)
1314113142
try:
1314213143
result = await gateway_service.transfer_gateway_ownership(
1314313144
db=db,
1314413145
gateway_id=gateway_id,
1314513146
target_owner_email=transfer.target_owner_email,
1314613147
actor_email=actor_email,
1314713148
target_team_id=transfer.target_team_id,
13149+
token_teams=token_teams,
1314813150
)
1314913151
return result
1315013152
except GatewayNotFoundError as e:
@@ -17980,6 +17982,7 @@ async def list_catalog_servers(
1798017982

1798117983
@admin_router.post("/mcp-registry/{server_id}/register", response_model=CatalogServerRegisterResponse)
1798217984
@require_permission("servers.create", allow_admin_bypass=False)
17985+
@require_permission("gateways.create", allow_admin_bypass=False)
1798317986
async def register_catalog_server(
1798417987
server_id: str,
1798517988
http_request: Request,
@@ -18119,6 +18122,7 @@ async def check_catalog_server_status(
1811918122

1812018123
@admin_router.post("/mcp-registry/bulk-register", response_model=CatalogBulkRegisterResponse)
1812118124
@require_permission("servers.create", allow_admin_bypass=False)
18125+
@require_permission("gateways.create", allow_admin_bypass=False)
1812218126
async def bulk_register_catalog_servers(
1812318127
http_request: Request,
1812418128
request: CatalogBulkRegisterRequest,

mcpgateway/middleware/token_scoping.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ class ResourceOwnershipResult(Enum):
208208
("POST", re.compile(r"^/admin/gateways/?$"), Permissions.GATEWAYS_CREATE),
209209
("POST", re.compile(r"^/admin/gateways/[^/]+/delete(?:$|/)"), Permissions.GATEWAYS_DELETE),
210210
("POST", re.compile(r"^/admin/gateways/[^/]+/(?:edit|state)(?:$|/)"), Permissions.GATEWAYS_UPDATE),
211+
("POST", re.compile(r"^/admin/gateways/[^/]+/transfer-ownership(?:$|/)"), Permissions.GATEWAYS_UPDATE),
211212
("GET", re.compile(r"^/admin/gateways(?:$|/)"), Permissions.GATEWAYS_READ),
212213
# Server management
213214
("POST", re.compile(r"^/admin/servers/?$"), Permissions.SERVERS_CREATE),

mcpgateway/services/gateway_service.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5529,6 +5529,7 @@ async def transfer_gateway_ownership(
55295529
target_owner_email: str,
55305530
actor_email: str,
55315531
target_team_id: Optional[str] = None,
5532+
token_teams: Optional[List[str]] = None,
55325533
) -> GatewayRead:
55335534
"""Transfer gateway ownership to another user.
55345535
@@ -5538,13 +5539,14 @@ async def transfer_gateway_ownership(
55385539
target_owner_email: Email of the new owner
55395540
actor_email: Email of the user performing the transfer
55405541
target_team_id: Optional new team ID for the gateway
5542+
token_teams: Normalized caller team scope; None means unrestricted admin scope
55415543
55425544
Returns:
55435545
GatewayRead with updated ownership
55445546
55455547
Raises:
55465548
GatewayNotFoundError: Gateway does not exist
5547-
ValueError: Target user invalid or team membership check fails
5549+
ValueError: Target user invalid, team membership check fails, or gateway is outside scope
55485550
"""
55495551
# Validate target user exists and is active
55505552
target_user = db.execute(select(DbEmailUser).where(DbEmailUser.email == target_owner_email, DbEmailUser.is_active == True)).scalar_one_or_none() # noqa: E712 # pylint: disable=singleton-comparison
@@ -5554,6 +5556,12 @@ async def transfer_gateway_ownership(
55545556
gateway = get_for_update(db, DbGateway, gateway_id)
55555557
if not gateway:
55565558
raise GatewayNotFoundError(f"Gateway not found: {gateway_id}")
5559+
if token_teams is not None:
5560+
current_team_id = gateway.team_id
5561+
if not current_team_id or current_team_id not in token_teams:
5562+
raise ValueError("Gateway is outside the caller's token team scope")
5563+
if target_team_id and target_team_id not in token_teams:
5564+
raise ValueError("Target team is outside the caller's token team scope")
55575565

55585566
previous_owner = gateway.owner_email
55595567
previous_team = gateway.team_id

tests/unit/mcpgateway/middleware/test_token_scoping.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,12 @@ def test_permission_restrictions_llm_proxy_default_prefix(self, middleware):
795795
assert middleware._check_permission_restrictions("/v1/chat/completions", "POST", [Permissions.LLM_INVOKE]) is True
796796
assert middleware._check_permission_restrictions("/v1/chat/completions", "POST", [Permissions.LLM_READ]) is False
797797

798+
def test_transfer_ownership_requires_gateway_update_permission(self, middleware):
799+
"""Ownership transfer must be covered by the gateway update token scope."""
800+
path = "/admin/gateways/gw-1/transfer-ownership"
801+
assert middleware._check_permission_restrictions(path, "POST", [Permissions.GATEWAYS_UPDATE]) is True
802+
assert middleware._check_permission_restrictions(path, "POST", [Permissions.GATEWAYS_READ]) is False
803+
798804
def test_permission_restrictions_llm_proxy_custom_prefix(self, middleware, monkeypatch):
799805
"""LLM proxy path mapping should follow settings.llm_api_prefix."""
800806
monkeypatch.setattr("mcpgateway.middleware.token_scoping.settings.llm_api_prefix", "/gateway-llm")

tests/unit/mcpgateway/services/test_gateway_service_transfer.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,3 +249,26 @@ async def test_transfer_private_gateway_to_team_coerces_visibility(service, mock
249249
assert tool.visibility == "team"
250250
assert resource.visibility == "team"
251251
assert prompt.visibility == "team"
252+
253+
254+
@pytest.mark.asyncio
255+
async def test_transfer_scoped_token_cannot_mutate_gateway_outside_scope(service, mock_db):
256+
"""Scoped administrators cannot transfer gateways outside their token teams."""
257+
gateway = MagicMock()
258+
gateway.visibility = "public"
259+
gateway.team_id = "foreign-team"
260+
gateway.owner_email = "old@example.com"
261+
262+
mock_db.execute.return_value = _scalar_result(MagicMock()) # target user found
263+
264+
with patch("mcpgateway.services.gateway_service.get_for_update", return_value=gateway):
265+
with pytest.raises(ValueError, match="outside.*scope"):
266+
await service.transfer_gateway_ownership(
267+
mock_db,
268+
"gw-1",
269+
"target@example.com",
270+
"actor@example.com",
271+
token_teams=["allowed-team"],
272+
)
273+
274+
mock_db.commit.assert_not_called()

tests/unit/mcpgateway/test_admin.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26702,6 +26702,32 @@ async def test_transfer_gateway_ownership_success(self, monkeypatch, allow_permi
2670226702
)
2670326703
assert result is expected
2670426704

26705+
@pytest.mark.asyncio
26706+
async def test_transfer_gateway_ownership_passes_token_team_scope(self, monkeypatch, allow_permission, mock_db):
26707+
"""Transfer route forwards the normalized token-team scope to the service."""
26708+
expected = MagicMock(spec=GatewayRead)
26709+
transfer_service = AsyncMock(return_value=expected)
26710+
monkeypatch.setattr("mcpgateway.admin.gateway_service.transfer_gateway_ownership", transfer_service)
26711+
monkeypatch.setattr("mcpgateway.admin.get_user_email", MagicMock(return_value="admin@x.com"))
26712+
26713+
result = await transfer_gateway_ownership(
26714+
gateway_id="gw-1",
26715+
transfer=GatewayOwnershipTransferRequest(target_owner_email="new@x.com", target_team_id="team-1"),
26716+
db=mock_db,
26717+
_user={"email": "admin@x.com", "is_admin": True, "token_teams": ["team-1"]},
26718+
)
26719+
26720+
assert result is expected
26721+
transfer_service.assert_awaited_once_with(
26722+
db=mock_db,
26723+
gateway_id="gw-1",
26724+
target_owner_email="new@x.com",
26725+
actor_email="admin@x.com",
26726+
target_team_id="team-1",
26727+
token_teams=["team-1"],
26728+
)
26729+
26730+
2670526731
@pytest.mark.asyncio
2670626732
async def test_transfer_gateway_ownership_not_found(self, monkeypatch, allow_permission, mock_db):
2670726733
monkeypatch.setattr(
@@ -26791,6 +26817,43 @@ async def test_admin_register_catalog_permission_error(self, monkeypatch, allow_
2679126817
await register_catalog_server("srv-1", request, db=mock_db, _user={"email": "admin@test.com"})
2679226818
assert exc_info.value.status_code == 403
2679326819

26820+
26821+
@pytest.mark.asyncio
26822+
async def test_admin_register_catalog_requires_gateways_create(self, monkeypatch, allow_permission, mock_db):
26823+
"""Catalog registration must require gateway creation permission too."""
26824+
monkeypatch.setattr("mcpgateway.admin.settings.mcpgateway_catalog_enabled", True, raising=False)
26825+
allow_permission.check_permission = AsyncMock(side_effect=[True, False])
26826+
monkeypatch.setattr(
26827+
"mcpgateway.admin.catalog_service.register_catalog_server",
26828+
AsyncMock(return_value=MagicMock()),
26829+
)
26830+
26831+
request = MagicMock(spec=Request)
26832+
request.headers = {}
26833+
with pytest.raises(HTTPException) as exc_info:
26834+
await register_catalog_server("srv-1", request, db=mock_db, _user={"email": "admin@test.com"})
26835+
26836+
assert exc_info.value.status_code == 403
26837+
26838+
@pytest.mark.asyncio
26839+
async def test_admin_bulk_register_catalog_requires_gateways_create(self, monkeypatch, allow_permission, mock_db):
26840+
"""Bulk catalog registration must require gateway creation permission too."""
26841+
monkeypatch.setattr("mcpgateway.admin.settings.mcpgateway_catalog_enabled", True, raising=False)
26842+
allow_permission.check_permission = AsyncMock(side_effect=[True, False])
26843+
26844+
from mcpgateway.schemas import CatalogBulkRegisterRequest
26845+
26846+
request = MagicMock(spec=Request)
26847+
with pytest.raises(HTTPException) as exc_info:
26848+
await bulk_register_catalog_servers(
26849+
request,
26850+
CatalogBulkRegisterRequest(server_ids=["srv-1"]),
26851+
db=mock_db,
26852+
_user={"email": "admin@test.com"},
26853+
)
26854+
26855+
assert exc_info.value.status_code == 403
26856+
2679426857
@pytest.mark.asyncio
2679526858
async def test_admin_bulk_register_catalog_permission_error(self, monkeypatch, allow_permission, mock_db):
2679626859
monkeypatch.setattr("mcpgateway.admin.settings.mcpgateway_catalog_enabled", True, raising=False)

0 commit comments

Comments
 (0)