Skip to content

Commit 45c897d

Browse files
authored
Mask auth for gateways APIs reponse (#602)
* masked auth values in gateways apis Signed-off-by: Keval Mahajan <[email protected]> * linting Signed-off-by: Keval Mahajan <[email protected]> * masked admin-gateways api auth values Signed-off-by: Keval Mahajan <[email protected]> * mask first and convert to the alias values Signed-off-by: Keval Mahajan <[email protected]> * linting Signed-off-by: Keval Mahajan <[email protected]> * flake8 issues resolved Signed-off-by: Keval Mahajan <[email protected]> * improved masking auth values Signed-off-by: Keval Mahajan <[email protected]> * updated test cases Signed-off-by: Keval Mahajan <[email protected]> * updated doctest for masked Signed-off-by: Keval Mahajan <[email protected]> * resolved flake8 issues Signed-off-by: Keval Mahajan <[email protected]> --------- Signed-off-by: Keval Mahajan <[email protected]>
1 parent ea4e3aa commit 45c897d

File tree

6 files changed

+142
-28
lines changed

6 files changed

+142
-28
lines changed

mcpgateway/admin.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,8 @@ async def admin_list_gateways(
10191019
... updated_at=datetime.now(timezone.utc),
10201020
... is_active=True,
10211021
... auth_type=None, auth_username=None, auth_password=None, auth_token=None,
1022-
... auth_header_key=None, auth_header_value=None
1022+
... auth_header_key=None, auth_header_value=None,
1023+
... slug="test-gateway"
10231024
... )
10241025
>>>
10251026
>>> # Mock the gateway_service.list_gateways method
@@ -1040,7 +1041,8 @@ async def admin_list_gateways(
10401041
... description="Another test", transport="HTTP", created_at=datetime.now(timezone.utc),
10411042
... updated_at=datetime.now(timezone.utc), enabled=False,
10421043
... auth_type=None, auth_username=None, auth_password=None, auth_token=None,
1043-
... auth_header_key=None, auth_header_value=None
1044+
... auth_header_key=None, auth_header_value=None,
1045+
... slug="test-gateway"
10441046
... )
10451047
>>> gateway_service.list_gateways = AsyncMock(return_value=[
10461048
... mock_gateway, # Return the GatewayRead objects, not pre-dumped dicts
@@ -2169,7 +2171,8 @@ async def admin_get_gateway(gateway_id: str, db: Session = Depends(get_db), user
21692171
... description="Gateway for getting", transport="HTTP",
21702172
... created_at=datetime.now(timezone.utc), updated_at=datetime.now(timezone.utc),
21712173
... enabled=True, auth_type=None, auth_username=None, auth_password=None,
2172-
... auth_token=None, auth_header_key=None, auth_header_value=None
2174+
... auth_token=None, auth_header_key=None, auth_header_value=None,
2175+
... slug="test-gateway"
21732176
... )
21742177
>>>
21752178
>>> # Mock the gateway_service.get_gateway method

mcpgateway/config.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,9 @@ def validate_database(self) -> None:
539539
# Rate limiting
540540
validation_max_requests_per_minute: int = 60
541541

542+
# Masking value for all sensitive data
543+
masked_auth_value: str = "*****"
544+
542545

543546
def extract_using_jq(data, jq_filter=""):
544547
"""

mcpgateway/schemas.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1882,6 +1882,7 @@ def _process_auth_fields(values: Dict[str, Any]) -> Optional[Dict[str, Any]]:
18821882
Raises:
18831883
ValueError: If auth type is invalid
18841884
"""
1885+
18851886
auth_type = values.data.get("auth_type")
18861887

18871888
if auth_type == "basic":
@@ -2032,6 +2033,11 @@ def _populate_auth(cls, values: Self) -> Dict[str, Any]:
20322033
"""
20332034
auth_type = values.auth_type
20342035
auth_value_encoded = values.auth_value
2036+
2037+
# Skip validation logic if masked value
2038+
if auth_value_encoded == settings.masked_auth_value:
2039+
return values
2040+
20352041
auth_value = decode_auth(auth_value_encoded)
20362042
if auth_type == "basic":
20372043
auth = auth_value.get("Authorization")
@@ -2056,6 +2062,40 @@ def _populate_auth(cls, values: Self) -> Dict[str, Any]:
20562062

20572063
return values
20582064

2065+
def masked(self) -> "GatewayRead":
2066+
"""
2067+
Return a masked version of the model instance with sensitive authentication fields hidden.
2068+
2069+
This method creates a dictionary representation of the model data and replaces sensitive fields
2070+
such as `auth_value`, `auth_password`, `auth_token`, and `auth_header_value` with a masked
2071+
placeholder value defined in `settings.masked_auth_value`. Masking is only applied if the fields
2072+
are present and not already masked.
2073+
2074+
Args:
2075+
None
2076+
2077+
Returns:
2078+
GatewayRead: A new instance of the GatewayRead model with sensitive authentication-related fields
2079+
masked to prevent exposure of sensitive information.
2080+
2081+
Notes:
2082+
- The `auth_value` field is only masked if it exists and its value is different from the masking
2083+
placeholder.
2084+
- Other sensitive fields (`auth_password`, `auth_token`, `auth_header_value`) are masked if present.
2085+
- Fields not related to authentication remain unchanged.
2086+
"""
2087+
masked_data = self.model_dump()
2088+
2089+
# Only mask if auth_value is present and not already masked
2090+
if masked_data.get("auth_value") and masked_data["auth_value"] != settings.masked_auth_value:
2091+
masked_data["auth_value"] = settings.masked_auth_value
2092+
2093+
masked_data["auth_password"] = settings.masked_auth_value if masked_data.get("auth_password") else None
2094+
masked_data["auth_token"] = settings.masked_auth_value if masked_data.get("auth_token") else None
2095+
masked_data["auth_header_value"] = settings.masked_auth_value if masked_data.get("auth_header_value") else None
2096+
2097+
return GatewayRead.model_validate(masked_data)
2098+
20592099

20602100
class FederatedTool(BaseModelWithConfigDict):
20612101
"""Schema for tools provided by federated gateways.

mcpgateway/services/gateway_service.py

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ async def register_gateway(self, db: Session, gateway: GatewayCreate) -> Gateway
386386
# Notify subscribers
387387
await self._notify_gateway_added(db_gateway)
388388

389-
return GatewayRead.model_validate(db_gateway)
389+
return GatewayRead.model_validate(db_gateway).masked()
390390
except* GatewayConnectionError as ge:
391391
if TYPE_CHECKING:
392392
ge: ExceptionGroup[GatewayConnectionError]
@@ -436,7 +436,9 @@ async def list_gateways(self, db: Session, include_inactive: bool = False) -> Li
436436
>>> db = MagicMock()
437437
>>> gateway_obj = MagicMock()
438438
>>> db.execute.return_value.scalars.return_value.all.return_value = [gateway_obj]
439-
>>> GatewayRead.model_validate = MagicMock(return_value='gateway_read')
439+
>>> mocked_gateway_read = MagicMock()
440+
>>> mocked_gateway_read.masked.return_value = 'gateway_read'
441+
>>> GatewayRead.model_validate = MagicMock(return_value=mocked_gateway_read)
440442
>>> import asyncio
441443
>>> result = asyncio.run(service.list_gateways(db))
442444
>>> result == ['gateway_read']
@@ -459,7 +461,7 @@ async def list_gateways(self, db: Session, include_inactive: bool = False) -> Li
459461
query = query.where(DbGateway.enabled)
460462

461463
gateways = db.execute(query).scalars().all()
462-
return [GatewayRead.model_validate(g) for g in gateways]
464+
return [GatewayRead.model_validate(g).masked() for g in gateways]
463465

464466
async def update_gateway(self, db: Session, gateway_id: str, gateway_update: GatewayUpdate, include_inactive: bool = True) -> GatewayRead:
465467
"""Update a gateway.
@@ -510,9 +512,20 @@ async def update_gateway(self, db: Session, gateway_id: str, gateway_update: Gat
510512
if getattr(gateway, "auth_type", None) is not None:
511513
gateway.auth_type = gateway_update.auth_type
512514

515+
# If auth_type is empty, update the auth_value too
516+
if gateway_update.auth_type == "":
517+
gateway.auth_value = ""
518+
513519
# if auth_type is not None and only then check auth_value
514-
if getattr(gateway, "auth_value", {}) != {}:
515-
gateway.auth_value = gateway_update.auth_value
520+
if getattr(gateway, "auth_value", "") != "":
521+
token = gateway_update.auth_token
522+
password = gateway_update.auth_password
523+
header_value = gateway_update.auth_header_value
524+
525+
if settings.masked_auth_value not in (token, password, header_value):
526+
# Check if values differ from existing ones
527+
if gateway.auth_value != gateway_update.auth_value:
528+
gateway.auth_value = gateway_update.auth_value
516529

517530
# Try to reinitialize connection if URL changed
518531
if gateway_update.url is not None:
@@ -557,7 +570,7 @@ async def update_gateway(self, db: Session, gateway_id: str, gateway_update: Gat
557570
await self._notify_gateway_updated(gateway)
558571

559572
logger.info(f"Updated gateway: {gateway.name}")
560-
return GatewayRead.model_validate(gateway)
573+
return GatewayRead.model_validate(gateway).masked()
561574

562575
except Exception as e:
563576
db.rollback()
@@ -578,15 +591,16 @@ async def get_gateway(self, db: Session, gateway_id: str, include_inactive: bool
578591
GatewayNotFoundError: If the gateway is not found
579592
580593
Examples:
581-
>>> from mcpgateway.services.gateway_service import GatewayService
582594
>>> from unittest.mock import MagicMock
583595
>>> from mcpgateway.schemas import GatewayRead
584596
>>> service = GatewayService()
585597
>>> db = MagicMock()
586598
>>> gateway_mock = MagicMock()
587599
>>> gateway_mock.enabled = True
588600
>>> db.get.return_value = gateway_mock
589-
>>> GatewayRead.model_validate = MagicMock(return_value='gateway_read')
601+
>>> mocked_gateway_read = MagicMock()
602+
>>> mocked_gateway_read.masked.return_value = 'gateway_read'
603+
>>> GatewayRead.model_validate = MagicMock(return_value=mocked_gateway_read)
590604
>>> import asyncio
591605
>>> result = asyncio.run(service.get_gateway(db, 'gateway_id'))
592606
>>> result == 'gateway_read'
@@ -620,7 +634,7 @@ async def get_gateway(self, db: Session, gateway_id: str, include_inactive: bool
620634
raise GatewayNotFoundError(f"Gateway not found: {gateway_id}")
621635

622636
if gateway.enabled or include_inactive:
623-
return GatewayRead.model_validate(gateway)
637+
return GatewayRead.model_validate(gateway).masked()
624638

625639
raise GatewayNotFoundError(f"Gateway not found: {gateway_id}")
626640

@@ -708,7 +722,7 @@ async def toggle_gateway_status(self, db: Session, gateway_id: str, activate: bo
708722

709723
logger.info(f"Gateway status: {gateway.name} - {'enabled' if activate else 'disabled'} and {'accessible' if reachable else 'inaccessible'}")
710724

711-
return GatewayRead.model_validate(gateway)
725+
return GatewayRead.model_validate(gateway).masked()
712726

713727
except Exception as e:
714728
db.rollback()

tests/unit/mcpgateway/services/test_gateway_service.py

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
# Standard
1919
from datetime import datetime, timezone
20-
from unittest.mock import AsyncMock, MagicMock, Mock
20+
from unittest.mock import AsyncMock, MagicMock, Mock, patch
2121

2222
# Third-Party
2323
import pytest
@@ -146,7 +146,7 @@ class TestGatewayService:
146146
# ────────────────────────────────────────────────────────────────────
147147

148148
@pytest.mark.asyncio
149-
async def test_register_gateway(self, gateway_service, test_db):
149+
async def test_register_gateway(self, gateway_service, test_db, monkeypatch):
150150
"""Successful gateway registration populates DB and returns data."""
151151
# DB: no gateway with that name; no existing tools found
152152
test_db.execute = Mock(
@@ -172,6 +172,18 @@ async def test_register_gateway(self, gateway_service, test_db):
172172
)
173173
gateway_service._notify_gateway_added = AsyncMock()
174174

175+
# Patch GatewayRead.model_validate to return a mock with .masked()
176+
mock_model = Mock()
177+
mock_model.masked.return_value = mock_model
178+
mock_model.name = "test_gateway"
179+
mock_model.url = "http://example.com/gateway"
180+
mock_model.description = "A test gateway"
181+
182+
monkeypatch.setattr(
183+
"mcpgateway.services.gateway_service.GatewayRead.model_validate",
184+
lambda x: mock_model,
185+
)
186+
175187
gateway_create = GatewayCreate(
176188
name="test_gateway",
177189
url="http://example.com/gateway",
@@ -236,10 +248,18 @@ async def test_register_gateway_connection_error(self, gateway_service, test_db)
236248
# ────────────────────────────────────────────────────────────────────
237249

238250
@pytest.mark.asyncio
239-
async def test_list_gateways(self, gateway_service, mock_gateway, test_db):
251+
async def test_list_gateways(self, gateway_service, mock_gateway, test_db, monkeypatch):
240252
"""Listing gateways returns the active ones."""
253+
241254
test_db.execute = Mock(return_value=_make_execute_result(scalars_list=[mock_gateway]))
242255

256+
mock_model = Mock()
257+
mock_model.masked.return_value = mock_model
258+
mock_model.name = "test_gateway"
259+
260+
# Patch using full path string to GatewayRead.model_validate
261+
monkeypatch.setattr("mcpgateway.services.gateway_service.GatewayRead.model_validate", lambda x: mock_model)
262+
243263
result = await gateway_service.list_gateways(test_db)
244264

245265
test_db.execute.assert_called_once()
@@ -249,6 +269,7 @@ async def test_list_gateways(self, gateway_service, mock_gateway, test_db):
249269
@pytest.mark.asyncio
250270
async def test_get_gateway(self, gateway_service, mock_gateway, test_db):
251271
"""Gateway is fetched and returned by ID."""
272+
mock_gateway.masked = Mock(return_value=mock_gateway)
252273
test_db.get = Mock(return_value=mock_gateway)
253274
result = await gateway_service.get_gateway(test_db, 1)
254275
test_db.get.assert_called_once_with(DbGateway, 1)
@@ -266,14 +287,24 @@ async def test_get_gateway_not_found(self, gateway_service, test_db):
266287
async def test_get_gateway_inactive(self, gateway_service, mock_gateway, test_db):
267288
"""Inactive gateway is not returned unless explicitly asked for."""
268289
mock_gateway.enabled = False
290+
mock_gateway.id = 1
269291
test_db.get = Mock(return_value=mock_gateway)
270-
result = await gateway_service.get_gateway(test_db, 1, include_inactive=True)
271-
assert result.id == 1
272-
assert result.enabled == False
273-
test_db.get.reset_mock()
274-
test_db.get = Mock(return_value=mock_gateway)
275-
with pytest.raises(GatewayNotFoundError):
276-
result = await gateway_service.get_gateway(test_db, 1, include_inactive=False)
292+
293+
# Create a mock for GatewayRead with a masked method
294+
mock_gateway_read = Mock()
295+
mock_gateway_read.id = 1
296+
mock_gateway_read.enabled = False
297+
mock_gateway_read.masked = Mock(return_value=mock_gateway_read)
298+
299+
with patch("mcpgateway.services.gateway_service.GatewayRead.model_validate", return_value=mock_gateway_read):
300+
result = await gateway_service.get_gateway(test_db, 1, include_inactive=True)
301+
assert result.id == 1
302+
assert result.enabled == False
303+
304+
# Now test the inactive = False path
305+
test_db.get = Mock(return_value=mock_gateway)
306+
with pytest.raises(GatewayNotFoundError):
307+
await gateway_service.get_gateway(test_db, 1, include_inactive=False)
277308

278309
# ────────────────────────────────────────────────────────────────────
279310
# UPDATE
@@ -288,22 +319,36 @@ async def test_update_gateway(self, gateway_service, mock_gateway, test_db):
288319
test_db.commit = Mock()
289320
test_db.refresh = Mock()
290321

322+
# Simulate successful gateway initialization
291323
gateway_service._initialize_gateway = AsyncMock(
292324
return_value=(
293-
{"prompts": {"subscribe": True}, "resources": {"subscribe": True}, "tools": {"subscribe": True}},
325+
{
326+
"prompts": {"subscribe": True},
327+
"resources": {"subscribe": True},
328+
"tools": {"subscribe": True},
329+
},
294330
[],
295331
)
296332
)
297333
gateway_service._notify_gateway_updated = AsyncMock()
298334

335+
# Create the update payload
299336
gateway_update = GatewayUpdate(
300337
name="updated_gateway",
301338
url="http://example.com/updated",
302339
description="Updated description",
303340
)
304341

305-
result = await gateway_service.update_gateway(test_db, 1, gateway_update)
342+
# Create mock return for GatewayRead.model_validate().masked()
343+
mock_gateway_read = MagicMock()
344+
mock_gateway_read.name = "updated_gateway"
345+
mock_gateway_read.masked.return_value = mock_gateway_read # Ensure .masked() returns the same object
346+
347+
# Patch the model_validate call in the service
348+
with patch("mcpgateway.services.gateway_service.GatewayRead.model_validate", return_value=mock_gateway_read):
349+
result = await gateway_service.update_gateway(test_db, 1, gateway_update)
306350

351+
# Assertions
307352
test_db.commit.assert_called_once()
308353
test_db.refresh.assert_called_once()
309354
gateway_service._initialize_gateway.assert_called_once()
@@ -354,6 +399,7 @@ async def test_toggle_gateway_status(self, gateway_service, mock_gateway, test_d
354399
query_proxy.filter.return_value = filter_proxy
355400
test_db.query = Mock(return_value=query_proxy)
356401

402+
# Setup gateway service mocks
357403
gateway_service._notify_gateway_activated = AsyncMock()
358404
gateway_service._notify_gateway_deactivated = AsyncMock()
359405
gateway_service._initialize_gateway = AsyncMock(return_value=({"prompts": {}}, []))
@@ -362,12 +408,17 @@ async def test_toggle_gateway_status(self, gateway_service, mock_gateway, test_d
362408
tool_service_stub.toggle_tool_status = AsyncMock()
363409
gateway_service.tool_service = tool_service_stub
364410

365-
result = await gateway_service.toggle_gateway_status(test_db, 1, activate=False)
411+
# Patch model_validate to return a mock with .masked()
412+
mock_gateway_read = MagicMock()
413+
mock_gateway_read.masked.return_value = mock_gateway_read
414+
415+
with patch("mcpgateway.services.gateway_service.GatewayRead.model_validate", return_value=mock_gateway_read):
416+
result = await gateway_service.toggle_gateway_status(test_db, 1, activate=False)
366417

367418
assert mock_gateway.enabled is False
368419
gateway_service._notify_gateway_deactivated.assert_called_once()
369420
assert tool_service_stub.toggle_tool_status.called
370-
assert result.enabled is False
421+
assert result == mock_gateway_read
371422

372423
# ────────────────────────────────────────────────────────────────────
373424
# DELETE

tests/unit/mcpgateway/test_admin.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,8 @@ async def test_admin_list_gateways_with_auth_info(self, mock_list_gateways, mock
771771
"transport": "HTTP",
772772
"enabled": True,
773773
"auth_type": "bearer",
774-
"auth_token": "hidden", # Should be masked
774+
"auth_token": "Bearer hidden", # Should be masked
775+
"auth_value": "Some value",
775776
}
776777

777778
mock_list_gateways.return_value = [mock_gateway]
@@ -789,6 +790,8 @@ async def test_admin_get_gateway_all_transports(self, mock_get_gateway, mock_db)
789790
mock_gateway.model_dump.return_value = {
790791
"id": f"gateway-{transport}",
791792
"transport": transport,
793+
"name": f"Gateway {transport}", # Add this field
794+
"url": f"https://gateway-{transport}.com", # Add this field
792795
}
793796
mock_get_gateway.return_value = mock_gateway
794797

0 commit comments

Comments
 (0)