From ff8a9a14c26bda87dec4acadd0ff3514e299f772 Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 17:26:56 +0530 Subject: [PATCH 01/11] imporved duplicated gateway check Signed-off-by: Keval Mahajan --- mcpgateway/admin.py | 4 +- mcpgateway/db.py | 2 +- mcpgateway/main.py | 10 +- mcpgateway/services/gateway_service.py | 244 ++++++++++++++---- .../services/test_gateway_service.py | 4 +- 5 files changed, 201 insertions(+), 63 deletions(-) diff --git a/mcpgateway/admin.py b/mcpgateway/admin.py index 9ef3cddc8..29d94e78c 100644 --- a/mcpgateway/admin.py +++ b/mcpgateway/admin.py @@ -102,7 +102,7 @@ from mcpgateway.services.catalog_service import catalog_service from mcpgateway.services.encryption_service import get_encryption_service from mcpgateway.services.export_service import ExportError, ExportService -from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayNameConflictError, GatewayNotFoundError, GatewayService, GatewayUrlConflictError +from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayNameConflictError, GatewayNotFoundError, GatewayService, GatewayDuplicateConflictError from mcpgateway.services.import_service import ConflictStrategy from mcpgateway.services.import_service import ImportError as ImportServiceError from mcpgateway.services.import_service import ImportService, ImportValidationError @@ -6846,7 +6846,7 @@ async def admin_add_gateway(request: Request, db: Session = Depends(get_db), use except GatewayConnectionError as ex: return JSONResponse(content={"message": str(ex), "success": False}, status_code=502) - except GatewayUrlConflictError as ex: + except GatewayDuplicateConflictError as ex: return JSONResponse(content={"message": str(ex), "success": False}, status_code=409) except GatewayNameConflictError as ex: return JSONResponse(content={"message": str(ex), "success": False}, status_code=409) diff --git a/mcpgateway/db.py b/mcpgateway/db.py index 3e06f65b4..4b37b0f17 100644 --- a/mcpgateway/db.py +++ b/mcpgateway/db.py @@ -2820,7 +2820,7 @@ class Gateway(Base): __table_args__ = ( UniqueConstraint("team_id", "owner_email", "slug", name="uq_team_owner_slug_gateway"), - UniqueConstraint("team_id", "owner_email", "url", name="uq_team_owner_url_gateway"), + # UniqueConstraint("team_id", "owner_email", "url", name="uq_team_owner_url_gateway"), ) diff --git a/mcpgateway/main.py b/mcpgateway/main.py index 20cf10896..55537fd77 100644 --- a/mcpgateway/main.py +++ b/mcpgateway/main.py @@ -108,7 +108,7 @@ from mcpgateway.services.a2a_service import A2AAgentError, A2AAgentNameConflictError, A2AAgentNotFoundError, A2AAgentService from mcpgateway.services.completion_service import CompletionService from mcpgateway.services.export_service import ExportError, ExportService -from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayError, GatewayNameConflictError, GatewayNotFoundError, GatewayService, GatewayUrlConflictError +from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayError, GatewayNameConflictError, GatewayNotFoundError, GatewayService, GatewayDuplicateConflictError from mcpgateway.services.import_service import ConflictStrategy, ImportConflictError from mcpgateway.services.import_service import ImportError as ImportServiceError from mcpgateway.services.import_service import ImportService, ImportValidationError @@ -3415,8 +3415,8 @@ async def register_gateway( return JSONResponse(content={"message": "Unable to process input"}, status_code=status.HTTP_400_BAD_REQUEST) if isinstance(ex, GatewayNameConflictError): return JSONResponse(content={"message": "Gateway name already exists"}, status_code=status.HTTP_409_CONFLICT) - if isinstance(ex, GatewayUrlConflictError): - return JSONResponse(content={"message": "Gateway URL already exists"}, status_code=status.HTTP_409_CONFLICT) + if isinstance(ex, GatewayDuplicateConflictError): + return JSONResponse(content={"message": "Gateway already exists"}, status_code=status.HTTP_409_CONFLICT) if isinstance(ex, RuntimeError): return JSONResponse(content={"message": "Error during execution"}, status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) if isinstance(ex, ValidationError): @@ -3493,8 +3493,8 @@ async def update_gateway( return JSONResponse(content={"message": "Unable to process input"}, status_code=status.HTTP_400_BAD_REQUEST) if isinstance(ex, GatewayNameConflictError): return JSONResponse(content={"message": "Gateway name already exists"}, status_code=status.HTTP_409_CONFLICT) - if isinstance(ex, GatewayUrlConflictError): - return JSONResponse(content={"message": "Gateway URL already exists"}, status_code=status.HTTP_409_CONFLICT) + if isinstance(ex, GatewayDuplicateConflictError): + return JSONResponse(content={"message": "Gateway already exists"}, status_code=status.HTTP_409_CONFLICT) if isinstance(ex, RuntimeError): return JSONResponse(content={"message": "Error during execution"}, status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) if isinstance(ex, ValidationError): diff --git a/mcpgateway/services/gateway_service.py b/mcpgateway/services/gateway_service.py index e69172e5e..fc8aed48c 100644 --- a/mcpgateway/services/gateway_service.py +++ b/mcpgateway/services/gateway_service.py @@ -177,54 +177,77 @@ def __init__(self, name: str, enabled: bool = True, gateway_id: Optional[int] = super().__init__(message) -class GatewayUrlConflictError(GatewayError): - """Raised when a gateway URL conflicts with existing (active or inactive) gateway. +class GatewayDuplicateConflictError(GatewayError): + """Raised when a gateway conflicts with existing gateway (same URL + credentials). + + This error is raised when attempting to register a gateway with a URL and + authentication credentials that already exist within the same scope: + - Public: Global uniqueness required + - Team: Uniqueness within the same team + - Private: No uniqueness check (this error won't be raised) Args: - url: The conflicting gateway URL - enabled: Whether the existing gateway is enabled - gateway_id: ID of the existing gateway if available - visibility: The visibility of the gateway ("public" or "team"). + duplicate_gateway: The existing conflicting gateway (DbGateway instance) + conflict_type: Type of credential conflict ("auth_value", "oauth_config", "no_auth") Examples: - >>> error = GatewayUrlConflictError("http://example.com/gateway") - >>> str(error) - 'Public Gateway already exists with URL: http://example.com/gateway' - >>> error.url - 'http://example.com/gateway' - >>> error.enabled - True - >>> error.gateway_id is None - True - - >>> error_inactive = GatewayUrlConflictError("http://inactive.com/gw", enabled=False, gateway_id=123) - >>> str(error_inactive) - 'Public Gateway already exists with URL: http://inactive.com/gw (currently inactive, ID: 123)' - >>> error_inactive.enabled - False - >>> error_inactive.gateway_id - 123 + >>> # Public gateway conflict with basic auth + >>> error = GatewayDuplicateConflictError( + ... duplicate_gateway=existing_gw, + ... conflict_type="auth_value" + ... ) + >>> str(error) + 'Gateway with URL "https://api.example.com" and same authentication credentials + already exists in public scope (ID: abc-123, Status: active, Owner: alice@example.com)' + + >>> # Team gateway conflict with OAuth + >>> error = GatewayDuplicateConflictError( + ... duplicate_gateway=team_gw, + ... conflict_type="oauth_config" + ... ) + >>> str(error) + 'Gateway with URL "https://api.example.com" and same OAuth configuration + already exists in team "engineering-team" (ID: def-456, Status: inactive, Owner: bob@example.com)' """ - def __init__(self, url: str, enabled: bool = True, gateway_id: Optional[int] = None, visibility: Optional[str] = "public"): + def __init__( + self, + duplicate_gateway: "DbGateway", + ): """Initialize the error with gateway information. Args: - url: The conflicting gateway URL - enabled: Whether the existing gateway is enabled - gateway_id: ID of the existing gateway if available - visibility: The visibility of the gateway ("public" or "team"). + duplicate_gateway: The existing conflicting gateway (DbGateway instance) """ - self.url = url - self.enabled = enabled - self.gateway_id = gateway_id - if visibility == "team": - vis_label = "Team-level" + self.duplicate_gateway = duplicate_gateway + self.url = duplicate_gateway.url + self.gateway_id = duplicate_gateway.id + self.enabled = duplicate_gateway.enabled + self.visibility = duplicate_gateway.visibility + self.team_id = duplicate_gateway.team_id + self.name = duplicate_gateway.name + + # Build scope description + if self.visibility == "public": + scope_desc = "public scope" + elif self.visibility == "team" and self.team_id: + scope_desc = f'team "{self.team_id}"' else: - vis_label = "Public" - message = f"{vis_label} Gateway already exists with URL: {url}" - if not enabled: - message += f" (currently inactive, ID: {gateway_id})" + scope_desc = f'"{self.visibility}" scope' + + # Build status description + status = "active" if self.enabled else "inactive" + + # Construct error message + message = ( + f'The Gateway with URL "{self.url}" already exists in {scope_desc} ' + f'(Name: {self.name}, Status: {status})' + ) + + # Add helpful hint for inactive gateways + if not self.enabled: + message += ". You may want to re-enable the existing gateway instead." + super().__init__(message) @@ -489,6 +512,92 @@ def _get_team_name(self, db: Session, team_id: Optional[str]) -> Optional[str]: team = db.query(EmailTeam).filter(EmailTeam.id == team_id, EmailTeam.is_active.is_(True)).first() return team.name if team else None + def _check_gateway_uniqueness( + self, + db: Session, + url: str, + auth_value: Optional[Dict[str, str]], + oauth_config: Optional[Dict[str, Any]], + team_id: Optional[str], + visibility: str, + gateway_id: Optional[str] = None + ) -> Optional[DbGateway]: + """ + Check if a gateway with the same URL and credentials already exists. + + Args: + db: Database session + url: Gateway URL (normalized) + auth_value: Decoded auth_value dict (not encrypted) + oauth_config: OAuth configuration dict + team_id: Team ID for team-scoped gateways + visibility: Gateway visibility (public/team/private) + gateway_id: Optional gateway ID to exclude from check (for updates) + + Returns: + DbGateway if duplicate found, None otherwise + """ + # Build base query based on visibility + if visibility == "public": + query = db.query(DbGateway).filter( + DbGateway.url == url, + DbGateway.visibility == "public" + ) + elif visibility == "team" and team_id: + query = db.query(DbGateway).filter( + DbGateway.url == url, + DbGateway.visibility == "team", + DbGateway.team_id == team_id + ) + else: # private + return None # Private gateways don't need cross-user uniqueness + + # Exclude current gateway if updating + if gateway_id: + query = query.filter(DbGateway.id != gateway_id) + + existing_gateways = query.all() + + # Check each existing gateway + for existing in existing_gateways: + # Case 1: Both have OAuth config + if oauth_config and existing.oauth_config: + # Compare OAuth configs (exclude dynamic fields like tokens) + existing_oauth = existing.oauth_config or {} + new_oauth = oauth_config or {} + + # Compare key OAuth fields + oauth_keys = ['grant_type', 'client_id', 'authorization_url', 'token_url', 'scope'] + if all(existing_oauth.get(k) == new_oauth.get(k) for k in oauth_keys): + return existing # Duplicate OAuth config found + + # Case 2: Both have auth_value (need to decrypt and compare) + elif auth_value and existing.auth_value: + + try: + # Decrypt existing auth_value + if isinstance(existing.auth_value, str): + existing_decoded = decode_auth(existing.auth_value) + + elif isinstance(existing.auth_value, dict): + existing_decoded = existing.auth_value + + else: + continue + + # Compare decoded auth values + if auth_value == existing_decoded: + return existing # Duplicate credentials found + except Exception as e: + logger.warning(f"Failed to decode auth_value for comparison: {e}") + continue + + # Case 3: Both have no auth (URL only, not allowed) + elif not auth_value and not oauth_config and not existing.auth_value and not existing.oauth_config: + return existing # Duplicate URL without credentials + + return None # No duplicate found + async def register_gateway( self, db: Session, @@ -568,17 +677,46 @@ async def register_gateway( # Normalize the gateway URL normalized_url = self.normalize_url(str(gateway.url)) - # Check for existing gateway with the same URL and visibility - if visibility.lower() == "public": - # Check for existing public gateway with the same URL - existing_gateway = db.execute(select(DbGateway).where(DbGateway.url == normalized_url, DbGateway.visibility == "public")).scalar_one_or_none() - if existing_gateway: - raise GatewayUrlConflictError(existing_gateway.url, enabled=existing_gateway.enabled, gateway_id=existing_gateway.id, visibility=existing_gateway.visibility) - elif visibility.lower() == "team" and team_id: - # Check for existing team gateway with the same URL - existing_gateway = db.execute(select(DbGateway).where(DbGateway.url == normalized_url, DbGateway.visibility == "team", DbGateway.team_id == team_id)).scalar_one_or_none() - if existing_gateway: - raise GatewayUrlConflictError(existing_gateway.url, enabled=existing_gateway.enabled, gateway_id=existing_gateway.id, visibility=existing_gateway.visibility) + + decoded_auth_value = None + if gateway.auth_value: + if isinstance(gateway.auth_value, str): + try: + decoded_auth_value = decode_auth(gateway.auth_value) + except Exception as e: + logger.warning(f"Failed to decode provided auth_value: {e}") + decoded_auth_value = None + elif isinstance(gateway.auth_value, dict): + decoded_auth_value = gateway.auth_value + + # Check for duplicate gateway + duplicate_gateway = self._check_gateway_uniqueness( + db=db, + url=normalized_url, + auth_value=decoded_auth_value, + oauth_config=gateway.oauth_config, + team_id=team_id, + visibility=visibility + ) + + if duplicate_gateway: + + error_msg = ( + f"The Gateway already exists " + f"(ID: {duplicate_gateway.id}, Name: {duplicate_gateway.name}, enabled: {duplicate_gateway.enabled})" + ) + + raise GatewayDuplicateConflictError( + duplicate_gateway = duplicate_gateway + ) + + # Prevent URL-only gateways (no auth at all) + # if not decoded_auth_value and not gateway.oauth_config: + # raise ValueError( + # f"Gateway with URL '{normalized_url}' must have either auth_value or oauth_config. " + # "URL-only gateways are not allowed." + # ) + auth_type = getattr(gateway, "auth_type", None) # Support multiple custom headers @@ -741,10 +879,10 @@ async def register_gateway( gnce: ExceptionGroup[GatewayNameConflictError] logger.error(f"GatewayNameConflictError in group: {gnce.exceptions}") raise gnce.exceptions[0] - except* GatewayUrlConflictError as guce: # pragma: no mutate + except* GatewayDuplicateConflictError as guce: # pragma: no mutate if TYPE_CHECKING: - guce: ExceptionGroup[GatewayUrlConflictError] - logger.error(f"GatewayUrlConflictError in group: {guce.exceptions}") + guce: ExceptionGroup[GatewayDuplicateConflictError] + logger.error(f"GatewayDuplicateConflictError in group: {guce.exceptions}") raise guce.exceptions[0] except* ValueError as ve: # pragma: no mutate if TYPE_CHECKING: @@ -1146,7 +1284,7 @@ async def update_gateway( if vis == "public": existing_gateway = db.execute(select(DbGateway).where(DbGateway.url == normalized_url, DbGateway.visibility == "public", DbGateway.id != gateway_id)).scalar_one_or_none() if existing_gateway: - raise GatewayUrlConflictError( + raise GatewayDuplicateConflictError( normalized_url, enabled=existing_gateway.enabled, gateway_id=existing_gateway.id, @@ -1157,7 +1295,7 @@ async def update_gateway( select(DbGateway).where(DbGateway.url == normalized_url, DbGateway.visibility == "team", DbGateway.team_id == gateway.team_id, DbGateway.id != gateway_id) ).scalar_one_or_none() if existing_gateway: - raise GatewayUrlConflictError( + raise GatewayDuplicateConflictError( normalized_url, enabled=existing_gateway.enabled, gateway_id=existing_gateway.id, diff --git a/tests/unit/mcpgateway/services/test_gateway_service.py b/tests/unit/mcpgateway/services/test_gateway_service.py index 6b17a02dc..b1a2a0e96 100644 --- a/tests/unit/mcpgateway/services/test_gateway_service.py +++ b/tests/unit/mcpgateway/services/test_gateway_service.py @@ -37,7 +37,7 @@ GatewayNameConflictError, GatewayNotFoundError, GatewayService, - GatewayUrlConflictError, + GatewayDuplicateConflictError, ) # --------------------------------------------------------------------------- @@ -559,7 +559,7 @@ async def test_register_gateway_with_existing_tools(self, gateway_service, test_ description="Gateway with existing tools", ) - with pytest.raises(GatewayUrlConflictError) as exc_info: + with pytest.raises(GatewayDuplicateConflictError) as exc_info: await gateway_service.register_gateway(test_db, gateway_create) err = exc_info.value From e6bb42e950f4986f4a6327ccd6d864cfed13feba Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 19:37:31 +0530 Subject: [PATCH 02/11] error message changes Signed-off-by: Keval Mahajan --- mcpgateway/services/gateway_service.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/mcpgateway/services/gateway_service.py b/mcpgateway/services/gateway_service.py index fc8aed48c..00dc35bcb 100644 --- a/mcpgateway/services/gateway_service.py +++ b/mcpgateway/services/gateway_service.py @@ -229,9 +229,9 @@ def __init__( # Build scope description if self.visibility == "public": - scope_desc = "public scope" + scope_desc = "Public scope" elif self.visibility == "team" and self.team_id: - scope_desc = f'team "{self.team_id}"' + scope_desc = f'the Team' else: scope_desc = f'"{self.visibility}" scope' @@ -240,7 +240,7 @@ def __init__( # Construct error message message = ( - f'The Gateway with URL "{self.url}" already exists in {scope_desc} ' + f'The Server already exists in {scope_desc} ' f'(Name: {self.name}, Status: {status})' ) @@ -701,11 +701,6 @@ async def register_gateway( if duplicate_gateway: - error_msg = ( - f"The Gateway already exists " - f"(ID: {duplicate_gateway.id}, Name: {duplicate_gateway.name}, enabled: {duplicate_gateway.enabled})" - ) - raise GatewayDuplicateConflictError( duplicate_gateway = duplicate_gateway ) From 6bd8fbfa6647d380ebecf3e76694ade2993cddfe Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 20:56:47 +0530 Subject: [PATCH 03/11] check gateway uniqueness while updating too Signed-off-by: Keval Mahajan --- mcpgateway/services/gateway_service.py | 76 +++++++++++++++++--------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/mcpgateway/services/gateway_service.py b/mcpgateway/services/gateway_service.py index 00dc35bcb..c20d4eb98 100644 --- a/mcpgateway/services/gateway_service.py +++ b/mcpgateway/services/gateway_service.py @@ -519,6 +519,7 @@ def _check_gateway_uniqueness( auth_value: Optional[Dict[str, str]], oauth_config: Optional[Dict[str, Any]], team_id: Optional[str], + owner_email: str, visibility: str, gateway_id: Optional[str] = None ) -> Optional[DbGateway]: @@ -531,6 +532,7 @@ def _check_gateway_uniqueness( auth_value: Decoded auth_value dict (not encrypted) oauth_config: OAuth configuration dict team_id: Team ID for team-scoped gateways + owner_email: Email of the gateway owner visibility: Gateway visibility (public/team/private) gateway_id: Optional gateway ID to exclude from check (for updates) @@ -549,8 +551,15 @@ def _check_gateway_uniqueness( DbGateway.visibility == "team", DbGateway.team_id == team_id ) - else: # private - return None # Private gateways don't need cross-user uniqueness + elif visibility == "private": + # Check for duplicates within the same user's private gateways + query = db.query(DbGateway).filter( + DbGateway.url == url, + DbGateway.visibility == "private", + DbGateway.owner_email == owner_email # Scoped to same user + ) + else: + return None # Exclude current gateway if updating if gateway_id: @@ -696,6 +705,7 @@ async def register_gateway( auth_value=decoded_auth_value, oauth_config=gateway.oauth_config, team_id=team_id, + owner_email=owner_email, visibility=visibility ) @@ -1272,30 +1282,44 @@ async def update_gateway( # Check for existing gateway with the same URL and visibility if gateway_update.url is not None: normalized_url = self.normalize_url(str(gateway_update.url)) - if gateway_update.visibility is not None: - vis = gateway_update.visibility - else: - vis = gateway.visibility - if vis == "public": - existing_gateway = db.execute(select(DbGateway).where(DbGateway.url == normalized_url, DbGateway.visibility == "public", DbGateway.id != gateway_id)).scalar_one_or_none() - if existing_gateway: - raise GatewayDuplicateConflictError( - normalized_url, - enabled=existing_gateway.enabled, - gateway_id=existing_gateway.id, - visibility=existing_gateway.visibility, - ) - elif vis == "team" and gateway.team_id: - existing_gateway = db.execute( - select(DbGateway).where(DbGateway.url == normalized_url, DbGateway.visibility == "team", DbGateway.team_id == gateway.team_id, DbGateway.id != gateway_id) - ).scalar_one_or_none() - if existing_gateway: - raise GatewayDuplicateConflictError( - normalized_url, - enabled=existing_gateway.enabled, - gateway_id=existing_gateway.id, - visibility=existing_gateway.visibility, - ) + + + # Prepare decoded auth_value for uniqueness check + decoded_auth_value = None + if gateway_update.auth_value: + if isinstance(gateway_update.auth_value, str): + try: + decoded_auth_value = decode_auth(gatewayupdate.auth_value) + except Exception as e: + logger.warning(f"Failed to decode provided auth_value: {e}") + elif isinstance(gateway_update.auth_value, dict): + decoded_auth_value = gateway_update.auth_value + + # Determine final values for uniqueness check + final_url = normalized_url + final_auth_value = decoded_auth_value if gateway_update.auth_value is not None else ( + decode_auth(gateway.auth_value) if isinstance(gateway.auth_value, str) else gateway.auth_value + ) + final_oauth_config = gateway_update.oauth_config if gateway_update.oauth_config is not None else gateway.oauth_config + final_visibility = gateway_update.visibility if gateway_update.visibility is not None else gateway.visibility + + # Check for duplicates with updated credentials + duplicate_gateway = self._check_gateway_uniqueness( + db=db, + url=final_url, + auth_value=final_auth_value, + oauth_config=final_oauth_config, + team_id=gateway.team_id, + visibility=final_visibility, + gateway_id=gateway_id, # Exclude current gateway from check + owner_email=user_email + ) + + if duplicate_gateway: + raise GatewayDuplicateConflictError( + duplicate_gateway=duplicate_gateway + ) + # FIX for Issue #1025: Determine if URL actually changed before we update it # We need this early because we update gateway.url below, and need to know From ae093317558b1e5ce4d93c8f8742d1a672e77705 Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 20:58:29 +0530 Subject: [PATCH 04/11] linting Signed-off-by: Keval Mahajan --- mcpgateway/admin.py | 2 +- mcpgateway/main.py | 2 +- mcpgateway/services/gateway_service.py | 101 +++++++++---------------- uv.lock | 6 +- 4 files changed, 41 insertions(+), 70 deletions(-) diff --git a/mcpgateway/admin.py b/mcpgateway/admin.py index 29d94e78c..2cb621d06 100644 --- a/mcpgateway/admin.py +++ b/mcpgateway/admin.py @@ -102,7 +102,7 @@ from mcpgateway.services.catalog_service import catalog_service from mcpgateway.services.encryption_service import get_encryption_service from mcpgateway.services.export_service import ExportError, ExportService -from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayNameConflictError, GatewayNotFoundError, GatewayService, GatewayDuplicateConflictError +from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayDuplicateConflictError, GatewayNameConflictError, GatewayNotFoundError, GatewayService from mcpgateway.services.import_service import ConflictStrategy from mcpgateway.services.import_service import ImportError as ImportServiceError from mcpgateway.services.import_service import ImportService, ImportValidationError diff --git a/mcpgateway/main.py b/mcpgateway/main.py index 55537fd77..cde579a10 100644 --- a/mcpgateway/main.py +++ b/mcpgateway/main.py @@ -108,7 +108,7 @@ from mcpgateway.services.a2a_service import A2AAgentError, A2AAgentNameConflictError, A2AAgentNotFoundError, A2AAgentService from mcpgateway.services.completion_service import CompletionService from mcpgateway.services.export_service import ExportError, ExportService -from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayError, GatewayNameConflictError, GatewayNotFoundError, GatewayService, GatewayDuplicateConflictError +from mcpgateway.services.gateway_service import GatewayConnectionError, GatewayDuplicateConflictError, GatewayError, GatewayNameConflictError, GatewayNotFoundError, GatewayService from mcpgateway.services.import_service import ConflictStrategy, ImportConflictError from mcpgateway.services.import_service import ImportError as ImportServiceError from mcpgateway.services.import_service import ImportService, ImportValidationError diff --git a/mcpgateway/services/gateway_service.py b/mcpgateway/services/gateway_service.py index c20d4eb98..89ca2e10e 100644 --- a/mcpgateway/services/gateway_service.py +++ b/mcpgateway/services/gateway_service.py @@ -180,7 +180,7 @@ def __init__(self, name: str, enabled: bool = True, gateway_id: Optional[int] = class GatewayDuplicateConflictError(GatewayError): """Raised when a gateway conflicts with existing gateway (same URL + credentials). - This error is raised when attempting to register a gateway with a URL and + This error is raised when attempting to register a gateway with a URL and authentication credentials that already exist within the same scope: - Public: Global uniqueness required - Team: Uniqueness within the same team @@ -197,7 +197,7 @@ class GatewayDuplicateConflictError(GatewayError): ... conflict_type="auth_value" ... ) >>> str(error) - 'Gateway with URL "https://api.example.com" and same authentication credentials + 'Gateway with URL "https://api.example.com" and same authentication credentials already exists in public scope (ID: abc-123, Status: active, Owner: alice@example.com)' >>> # Team gateway conflict with OAuth @@ -206,12 +206,12 @@ class GatewayDuplicateConflictError(GatewayError): ... conflict_type="oauth_config" ... ) >>> str(error) - 'Gateway with URL "https://api.example.com" and same OAuth configuration + 'Gateway with URL "https://api.example.com" and same OAuth configuration already exists in team "engineering-team" (ID: def-456, Status: inactive, Owner: bob@example.com)' """ def __init__( - self, + self, duplicate_gateway: "DbGateway", ): """Initialize the error with gateway information. @@ -231,7 +231,7 @@ def __init__( if self.visibility == "public": scope_desc = "Public scope" elif self.visibility == "team" and self.team_id: - scope_desc = f'the Team' + scope_desc = f"the Team" else: scope_desc = f'"{self.visibility}" scope' @@ -239,10 +239,7 @@ def __init__( status = "active" if self.enabled else "inactive" # Construct error message - message = ( - f'The Server already exists in {scope_desc} ' - f'(Name: {self.name}, Status: {status})' - ) + message = f"The Server already exists in {scope_desc} " f"(Name: {self.name}, Status: {status})" # Add helpful hint for inactive gateways if not self.enabled: @@ -513,19 +510,19 @@ def _get_team_name(self, db: Session, team_id: Optional[str]) -> Optional[str]: return team.name if team else None def _check_gateway_uniqueness( - self, - db: Session, - url: str, - auth_value: Optional[Dict[str, str]], + self, + db: Session, + url: str, + auth_value: Optional[Dict[str, str]], oauth_config: Optional[Dict[str, Any]], team_id: Optional[str], - owner_email: str, + owner_email: str, visibility: str, - gateway_id: Optional[str] = None + gateway_id: Optional[str] = None, ) -> Optional[DbGateway]: """ Check if a gateway with the same URL and credentials already exists. - + Args: db: Database session url: Gateway URL (normalized) @@ -535,36 +532,25 @@ def _check_gateway_uniqueness( owner_email: Email of the gateway owner visibility: Gateway visibility (public/team/private) gateway_id: Optional gateway ID to exclude from check (for updates) - + Returns: DbGateway if duplicate found, None otherwise """ # Build base query based on visibility if visibility == "public": - query = db.query(DbGateway).filter( - DbGateway.url == url, - DbGateway.visibility == "public" - ) + query = db.query(DbGateway).filter(DbGateway.url == url, DbGateway.visibility == "public") elif visibility == "team" and team_id: - query = db.query(DbGateway).filter( - DbGateway.url == url, - DbGateway.visibility == "team", - DbGateway.team_id == team_id - ) + query = db.query(DbGateway).filter(DbGateway.url == url, DbGateway.visibility == "team", DbGateway.team_id == team_id) elif visibility == "private": # Check for duplicates within the same user's private gateways - query = db.query(DbGateway).filter( - DbGateway.url == url, - DbGateway.visibility == "private", - DbGateway.owner_email == owner_email # Scoped to same user - ) + query = db.query(DbGateway).filter(DbGateway.url == url, DbGateway.visibility == "private", DbGateway.owner_email == owner_email) # Scoped to same user else: return None - + # Exclude current gateway if updating if gateway_id: query = query.filter(DbGateway.id != gateway_id) - + existing_gateways = query.all() # Check each existing gateway @@ -574,37 +560,37 @@ def _check_gateway_uniqueness( # Compare OAuth configs (exclude dynamic fields like tokens) existing_oauth = existing.oauth_config or {} new_oauth = oauth_config or {} - + # Compare key OAuth fields - oauth_keys = ['grant_type', 'client_id', 'authorization_url', 'token_url', 'scope'] + oauth_keys = ["grant_type", "client_id", "authorization_url", "token_url", "scope"] if all(existing_oauth.get(k) == new_oauth.get(k) for k in oauth_keys): return existing # Duplicate OAuth config found - + # Case 2: Both have auth_value (need to decrypt and compare) elif auth_value and existing.auth_value: - + try: # Decrypt existing auth_value if isinstance(existing.auth_value, str): existing_decoded = decode_auth(existing.auth_value) - + elif isinstance(existing.auth_value, dict): existing_decoded = existing.auth_value - + else: continue - + # Compare decoded auth values if auth_value == existing_decoded: return existing # Duplicate credentials found except Exception as e: logger.warning(f"Failed to decode auth_value for comparison: {e}") continue - + # Case 3: Both have no auth (URL only, not allowed) elif not auth_value and not oauth_config and not existing.auth_value and not existing.oauth_config: return existing # Duplicate URL without credentials - + return None # No duplicate found async def register_gateway( @@ -686,7 +672,7 @@ async def register_gateway( # Normalize the gateway URL normalized_url = self.normalize_url(str(gateway.url)) - + decoded_auth_value = None if gateway.auth_value: if isinstance(gateway.auth_value, str): @@ -698,22 +684,14 @@ async def register_gateway( elif isinstance(gateway.auth_value, dict): decoded_auth_value = gateway.auth_value - # Check for duplicate gateway + # Check for duplicate gateway duplicate_gateway = self._check_gateway_uniqueness( - db=db, - url=normalized_url, - auth_value=decoded_auth_value, - oauth_config=gateway.oauth_config, - team_id=team_id, - owner_email=owner_email, - visibility=visibility + db=db, url=normalized_url, auth_value=decoded_auth_value, oauth_config=gateway.oauth_config, team_id=team_id, owner_email=owner_email, visibility=visibility ) if duplicate_gateway: - - raise GatewayDuplicateConflictError( - duplicate_gateway = duplicate_gateway - ) + + raise GatewayDuplicateConflictError(duplicate_gateway=duplicate_gateway) # Prevent URL-only gateways (no auth at all) # if not decoded_auth_value and not gateway.oauth_config: @@ -722,7 +700,6 @@ async def register_gateway( # "URL-only gateways are not allowed." # ) - auth_type = getattr(gateway, "auth_type", None) # Support multiple custom headers auth_value = getattr(gateway, "auth_value", {}) @@ -1283,7 +1260,6 @@ async def update_gateway( if gateway_update.url is not None: normalized_url = self.normalize_url(str(gateway_update.url)) - # Prepare decoded auth_value for uniqueness check decoded_auth_value = None if gateway_update.auth_value: @@ -1297,9 +1273,7 @@ async def update_gateway( # Determine final values for uniqueness check final_url = normalized_url - final_auth_value = decoded_auth_value if gateway_update.auth_value is not None else ( - decode_auth(gateway.auth_value) if isinstance(gateway.auth_value, str) else gateway.auth_value - ) + final_auth_value = decoded_auth_value if gateway_update.auth_value is not None else (decode_auth(gateway.auth_value) if isinstance(gateway.auth_value, str) else gateway.auth_value) final_oauth_config = gateway_update.oauth_config if gateway_update.oauth_config is not None else gateway.oauth_config final_visibility = gateway_update.visibility if gateway_update.visibility is not None else gateway.visibility @@ -1312,14 +1286,11 @@ async def update_gateway( team_id=gateway.team_id, visibility=final_visibility, gateway_id=gateway_id, # Exclude current gateway from check - owner_email=user_email + owner_email=user_email, ) if duplicate_gateway: - raise GatewayDuplicateConflictError( - duplicate_gateway=duplicate_gateway - ) - + raise GatewayDuplicateConflictError(duplicate_gateway=duplicate_gateway) # FIX for Issue #1025: Determine if URL actually changed before we update it # We need this early because we update gateway.url below, and need to know diff --git a/uv.lock b/uv.lock index 249c2a239..ead203ae2 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 2 +revision = 3 requires-python = ">=3.11, <3.14" resolution-markers = [ "python_full_version >= '3.13' and platform_machine == 'x86_64' and sys_platform == 'darwin'", @@ -3131,8 +3131,8 @@ requires-dist = [ { name = "langchain-openai", marker = "extra == 'llmchat'", specifier = ">=1.0.2" }, { name = "langgraph", marker = "extra == 'llmchat'", specifier = ">=1.0.2" }, { name = "mcp", specifier = ">=1.21.0" }, - { name = "mcp-contextforge-gateway", extras = ["redis"], marker = "extra == 'all'", specifier = ">=0.8.0" }, - { name = "mcp-contextforge-gateway", extras = ["redis", "dev"], marker = "extra == 'dev-all'", specifier = ">=0.8.0" }, + { name = "mcp-contextforge-gateway", extras = ["redis"], marker = "extra == 'all'", specifier = ">=0.9.0" }, + { name = "mcp-contextforge-gateway", extras = ["redis", "dev"], marker = "extra == 'dev-all'", specifier = ">=0.9.0" }, { name = "oauthlib", specifier = ">=3.3.1" }, { name = "opentelemetry-api", marker = "extra == 'observability'", specifier = ">=1.38.0" }, { name = "opentelemetry-sdk", marker = "extra == 'observability'", specifier = ">=1.38.0" }, From 7585511453a945d16746c484d6cf1a0a0986ccfe Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 21:06:53 +0530 Subject: [PATCH 05/11] code linting Signed-off-by: Keval Mahajan --- mcpgateway/services/gateway_service.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/mcpgateway/services/gateway_service.py b/mcpgateway/services/gateway_service.py index 89ca2e10e..3b38f08cf 100644 --- a/mcpgateway/services/gateway_service.py +++ b/mcpgateway/services/gateway_service.py @@ -231,7 +231,7 @@ def __init__( if self.visibility == "public": scope_desc = "Public scope" elif self.visibility == "team" and self.team_id: - scope_desc = f"the Team" + scope_desc = "the Team" else: scope_desc = f'"{self.visibility}" scope' @@ -1257,22 +1257,24 @@ async def update_gateway( visibility=existing_gateway.visibility, ) # Check for existing gateway with the same URL and visibility + normalized_url = "" if gateway_update.url is not None: normalized_url = self.normalize_url(str(gateway_update.url)) + else: + normalized_url = None # Prepare decoded auth_value for uniqueness check decoded_auth_value = None if gateway_update.auth_value: if isinstance(gateway_update.auth_value, str): try: - decoded_auth_value = decode_auth(gatewayupdate.auth_value) + decoded_auth_value = decode_auth(gateway_update.auth_value) except Exception as e: logger.warning(f"Failed to decode provided auth_value: {e}") elif isinstance(gateway_update.auth_value, dict): decoded_auth_value = gateway_update.auth_value # Determine final values for uniqueness check - final_url = normalized_url final_auth_value = decoded_auth_value if gateway_update.auth_value is not None else (decode_auth(gateway.auth_value) if isinstance(gateway.auth_value, str) else gateway.auth_value) final_oauth_config = gateway_update.oauth_config if gateway_update.oauth_config is not None else gateway.oauth_config final_visibility = gateway_update.visibility if gateway_update.visibility is not None else gateway.visibility @@ -1280,7 +1282,7 @@ async def update_gateway( # Check for duplicates with updated credentials duplicate_gateway = self._check_gateway_uniqueness( db=db, - url=final_url, + url=normalized_url, auth_value=final_auth_value, oauth_config=final_oauth_config, team_id=gateway.team_id, From 567a0b693bcb57ec12748737bd7462f30249d622 Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 22:44:50 +0530 Subject: [PATCH 06/11] added alembic migration script for removal of url uniquess constraint Signed-off-by: Keval Mahajan --- ...b8_remove_gateway_url_unique_constraint.py | 96 +++++++++++++++++++ mcpgateway/db.py | 1 - 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py diff --git a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py new file mode 100644 index 000000000..e9a38f4fd --- /dev/null +++ b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py @@ -0,0 +1,96 @@ +# -*- coding: utf-8 -*- +"""Location: ./mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py +Copyright 2025 +SPDX-License-Identifier: Apache-2.0 +Authors: Keval Mahajan + +Alembic migration to remove unique constraint on gateway URL. +An improved alternative duplication check has been implemented for gateway duplication prevention. + +Revision ID: f3a3a3d901b8 +Revises: aac21d6f9522 +Create Date: 2025-11-11 22:30:05.474282 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.engine import Inspector + + + +# revision identifiers, used by Alembic. +revision: str = 'f3a3a3d901b8' +down_revision: Union[str, Sequence[str], None] = 'aac21d6f9522' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def constraint_exists(inspector, table_name, constraint_name): + """Check if a constraint exists in the table.""" + try: + unique_constraints = inspector.get_unique_constraints(table_name) + return any(uc['name'] == constraint_name for uc in unique_constraints) + except Exception: + # Fallback: assume constraint exists if we can't check + return True + + +def upgrade(): + """Remove the unique constraint on (team_id, owner_email, url) from gateway table.""" + + conn = op.get_bind() + inspector = Inspector.from_engine(conn) + + # Check if constraint exists before attempting to drop + if not constraint_exists(inspector, 'gateways', 'uq_team_owner_url_gateway'): + print("Constraint 'uq_team_owner_url_gateway' does not exist, skipping drop.") + return + + if conn.dialect.name == 'sqlite': + # SQLite: Use batch mode to recreate table without the constraint + with op.batch_alter_table('gateways', schema=None) as batch_op: + batch_op.drop_constraint( + 'uq_team_owner_url_gateway', + type_='unique' + ) + else: + # PostgreSQL, MySQL, etc.: Direct constraint drop + op.drop_constraint( + 'uq_team_owner_url_gateway', + 'gateways', + type_='unique' + ) + + print("Successfully removed constraint 'uq_team_owner_url_gateway' from gateway table.") + + +def downgrade(): + """Re-add the unique constraint on (team_id, owner_email, url) to gateway table.""" + + conn = op.get_bind() + inspector = Inspector.from_engine(conn) + + # Check if constraint already exists before attempting to create + if constraint_exists(inspector, 'gateways', 'uq_team_owner_url_gateway'): + print("Constraint 'uq_team_owner_url_gateway' already exists, skipping creation.") + return + + if conn.dialect.name == 'sqlite': + # SQLite: Use batch mode to recreate table with the constraint + with op.batch_alter_table('gateways', schema=None) as batch_op: + batch_op.create_unique_constraint( + 'uq_team_owner_url_gateway', + ['team_id', 'owner_email', 'url'] + ) + else: + # PostgreSQL, MySQL, etc.: Direct constraint creation + op.create_unique_constraint( + 'uq_team_owner_url_constraint', + 'gateways', + ['team_id', 'owner_email', 'url'] + ) + + print("Successfully re-added constraint 'uq_team_owner_url_gateway' to gateways table.") diff --git a/mcpgateway/db.py b/mcpgateway/db.py index 4b37b0f17..bb176b65a 100644 --- a/mcpgateway/db.py +++ b/mcpgateway/db.py @@ -2820,7 +2820,6 @@ class Gateway(Base): __table_args__ = ( UniqueConstraint("team_id", "owner_email", "slug", name="uq_team_owner_slug_gateway"), - # UniqueConstraint("team_id", "owner_email", "url", name="uq_team_owner_url_gateway"), ) From 27648fd1d5b538d19ef3e1e2f43baa810ee14aca Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Tue, 11 Nov 2025 22:46:00 +0530 Subject: [PATCH 07/11] lints Signed-off-by: Keval Mahajan --- ...b8_remove_gateway_url_unique_constraint.py | 62 +++++++------------ mcpgateway/db.py | 4 +- 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py index e9a38f4fd..7e20c3c14 100644 --- a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py +++ b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py @@ -4,7 +4,7 @@ SPDX-License-Identifier: Apache-2.0 Authors: Keval Mahajan -Alembic migration to remove unique constraint on gateway URL. +Alembic migration to remove unique constraint on gateway URL. An improved alternative duplication check has been implemented for gateway duplication prevention. Revision ID: f3a3a3d901b8 @@ -13,17 +13,17 @@ """ +# Standard from typing import Sequence, Union +# Third-Party from alembic import op import sqlalchemy as sa from sqlalchemy.engine import Inspector - - # revision identifiers, used by Alembic. -revision: str = 'f3a3a3d901b8' -down_revision: Union[str, Sequence[str], None] = 'aac21d6f9522' +revision: str = "f3a3a3d901b8" +down_revision: Union[str, Sequence[str], None] = "aac21d6f9522" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None @@ -32,7 +32,7 @@ def constraint_exists(inspector, table_name, constraint_name): """Check if a constraint exists in the table.""" try: unique_constraints = inspector.get_unique_constraints(table_name) - return any(uc['name'] == constraint_name for uc in unique_constraints) + return any(uc["name"] == constraint_name for uc in unique_constraints) except Exception: # Fallback: assume constraint exists if we can't check return True @@ -40,57 +40,43 @@ def constraint_exists(inspector, table_name, constraint_name): def upgrade(): """Remove the unique constraint on (team_id, owner_email, url) from gateway table.""" - + conn = op.get_bind() inspector = Inspector.from_engine(conn) - + # Check if constraint exists before attempting to drop - if not constraint_exists(inspector, 'gateways', 'uq_team_owner_url_gateway'): + if not constraint_exists(inspector, "gateways", "uq_team_owner_url_gateway"): print("Constraint 'uq_team_owner_url_gateway' does not exist, skipping drop.") return - - if conn.dialect.name == 'sqlite': + + if conn.dialect.name == "sqlite": # SQLite: Use batch mode to recreate table without the constraint - with op.batch_alter_table('gateways', schema=None) as batch_op: - batch_op.drop_constraint( - 'uq_team_owner_url_gateway', - type_='unique' - ) + with op.batch_alter_table("gateways", schema=None) as batch_op: + batch_op.drop_constraint("uq_team_owner_url_gateway", type_="unique") else: # PostgreSQL, MySQL, etc.: Direct constraint drop - op.drop_constraint( - 'uq_team_owner_url_gateway', - 'gateways', - type_='unique' - ) - + op.drop_constraint("uq_team_owner_url_gateway", "gateways", type_="unique") + print("Successfully removed constraint 'uq_team_owner_url_gateway' from gateway table.") def downgrade(): """Re-add the unique constraint on (team_id, owner_email, url) to gateway table.""" - + conn = op.get_bind() inspector = Inspector.from_engine(conn) - + # Check if constraint already exists before attempting to create - if constraint_exists(inspector, 'gateways', 'uq_team_owner_url_gateway'): + if constraint_exists(inspector, "gateways", "uq_team_owner_url_gateway"): print("Constraint 'uq_team_owner_url_gateway' already exists, skipping creation.") return - - if conn.dialect.name == 'sqlite': + + if conn.dialect.name == "sqlite": # SQLite: Use batch mode to recreate table with the constraint - with op.batch_alter_table('gateways', schema=None) as batch_op: - batch_op.create_unique_constraint( - 'uq_team_owner_url_gateway', - ['team_id', 'owner_email', 'url'] - ) + with op.batch_alter_table("gateways", schema=None) as batch_op: + batch_op.create_unique_constraint("uq_team_owner_url_gateway", ["team_id", "owner_email", "url"]) else: # PostgreSQL, MySQL, etc.: Direct constraint creation - op.create_unique_constraint( - 'uq_team_owner_url_constraint', - 'gateways', - ['team_id', 'owner_email', 'url'] - ) - + op.create_unique_constraint("uq_team_owner_url_constraint", "gateways", ["team_id", "owner_email", "url"]) + print("Successfully re-added constraint 'uq_team_owner_url_gateway' to gateways table.") diff --git a/mcpgateway/db.py b/mcpgateway/db.py index bb176b65a..7fd654ba0 100644 --- a/mcpgateway/db.py +++ b/mcpgateway/db.py @@ -2818,9 +2818,7 @@ class Gateway(Base): registered_oauth_clients: Mapped[List["RegisteredOAuthClient"]] = relationship("RegisteredOAuthClient", back_populates="gateway", cascade="all, delete-orphan") - __table_args__ = ( - UniqueConstraint("team_id", "owner_email", "slug", name="uq_team_owner_slug_gateway"), - ) + __table_args__ = (UniqueConstraint("team_id", "owner_email", "slug", name="uq_team_owner_slug_gateway"),) @event.listens_for(Gateway, "after_update") From 3495b22b0a389b8d61cf097296ab8ee9e8666aba Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Wed, 12 Nov 2025 10:35:09 +0530 Subject: [PATCH 08/11] updated doctest Signed-off-by: Keval Mahajan --- mcpgateway/services/gateway_service.py | 39 +++++++++++++++----------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/mcpgateway/services/gateway_service.py b/mcpgateway/services/gateway_service.py index 3b38f08cf..e2b24141c 100644 --- a/mcpgateway/services/gateway_service.py +++ b/mcpgateway/services/gateway_service.py @@ -178,36 +178,41 @@ def __init__(self, name: str, enabled: bool = True, gateway_id: Optional[int] = class GatewayDuplicateConflictError(GatewayError): - """Raised when a gateway conflicts with existing gateway (same URL + credentials). + """Raised when a gateway conflicts with an existing gateway (same URL + credentials). This error is raised when attempting to register a gateway with a URL and authentication credentials that already exist within the same scope: - - Public: Global uniqueness required - - Team: Uniqueness within the same team - - Private: No uniqueness check (this error won't be raised) + - Public: Global uniqueness required across all public gateways. + - Team: Uniqueness required within the same team. + - Private: Uniqueness required for the same user, a user cannot have two private gateways with the same URL and credentials. Args: - duplicate_gateway: The existing conflicting gateway (DbGateway instance) - conflict_type: Type of credential conflict ("auth_value", "oauth_config", "no_auth") + duplicate_gateway: The existing conflicting gateway (DbGateway instance). Examples: - >>> # Public gateway conflict with basic auth + >>> # Public gateway conflict with the same URL and basic auth + >>> existing_gw = DbGateway(url="https://api.example.com", id="abc-123", enabled=True, visibility="public", team_id=None, name="API Gateway", owner_email="alice@example.com") >>> error = GatewayDuplicateConflictError( - ... duplicate_gateway=existing_gw, - ... conflict_type="auth_value" + ... duplicate_gateway=existing_gw ... ) >>> str(error) - 'Gateway with URL "https://api.example.com" and same authentication credentials - already exists in public scope (ID: abc-123, Status: active, Owner: alice@example.com)' + 'The Server already exists in Public scope (Name: API Gateway, Status: active)' - >>> # Team gateway conflict with OAuth + >>> # Team gateway conflict with the same URL and OAuth credentials + >>> team_gw = DbGateway(url="https://api.example.com", id="def-456", enabled=False, visibility="team", team_id="engineering-team", name="API Gateway", owner_email="bob@example.com") >>> error = GatewayDuplicateConflictError( - ... duplicate_gateway=team_gw, - ... conflict_type="oauth_config" + ... duplicate_gateway=team_gw ... ) >>> str(error) - 'Gateway with URL "https://api.example.com" and same OAuth configuration - already exists in team "engineering-team" (ID: def-456, Status: inactive, Owner: bob@example.com)' + 'The Server already exists in your Team (Name: API Gateway, Status: inactive). You may want to re-enable the existing gateway instead.' + + >>> # Private gateway conflict (same user cannot have two gateways with the same URL) + >>> private_gw = DbGateway(url="https://api.example.com", id="ghi-789", enabled=True, visibility="private", team_id="none", name="API Gateway", owner_email="charlie@example.com") + >>> error = GatewayDuplicateConflictError( + ... duplicate_gateway=private_gw + ... ) + >>> str(error) + 'The Server already exists in "private" scope (Name: API Gateway, Status: active)' """ def __init__( @@ -231,7 +236,7 @@ def __init__( if self.visibility == "public": scope_desc = "Public scope" elif self.visibility == "team" and self.team_id: - scope_desc = "the Team" + scope_desc = "your Team" else: scope_desc = f'"{self.visibility}" scope' From dfb068e327a0d225b148799206d163b94cb6381a Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Wed, 12 Nov 2025 10:35:38 +0530 Subject: [PATCH 09/11] updated test cases Signed-off-by: Keval Mahajan --- .../services/test_gateway_service.py | 52 +++++++------------ 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/tests/unit/mcpgateway/services/test_gateway_service.py b/tests/unit/mcpgateway/services/test_gateway_service.py index b1a2a0e96..078e2b16b 100644 --- a/tests/unit/mcpgateway/services/test_gateway_service.py +++ b/tests/unit/mcpgateway/services/test_gateway_service.py @@ -516,56 +516,44 @@ async def test_register_gateway_exception_rollback(self, gateway_service, test_d @pytest.mark.asyncio async def test_register_gateway_with_existing_tools(self, gateway_service, test_db, monkeypatch): - """Test registering gateway with tools that already exist in database.""" - # Mock existing tool in database - existing_tool = MagicMock() - existing_tool.original_name = "existing_tool" - existing_tool.id = 123 - existing_tool.url = "http://example.com/gateway" - existing_tool.enabled = True - existing_tool.visibility = "public" + """Test registering gateway with URL/credentials that already exist (duplicate gateway).""" + # Mock existing GATEWAY in database (not tool) + existing_gateway = MagicMock() + existing_gateway.id = 123 + existing_gateway.url = "http://example.com/gateway" + existing_gateway.enabled = True + existing_gateway.visibility = "public" + existing_gateway.name = "existing_gateway" + existing_gateway.team_id = None + existing_gateway.owner_email = "test@example.com" test_db.execute = Mock( side_effect=[ _make_execute_result(scalar=None), # name-conflict check - _make_execute_result(scalar=existing_tool), # existing tool found + # No second call needed - check_gateway_uniqueness uses query().all() ] ) + + # Mock check_gateway_uniqueness to return the existing gateway + gateway_service._check_gateway_uniqueness = Mock(return_value=existing_gateway) + test_db.add = Mock() test_db.commit = Mock() test_db.refresh = Mock() - # Mock tools returned from gateway - # First-Party - from mcpgateway.schemas import ToolCreate - - mock_tools = [ToolCreate(name="existing_tool", description="An existing tool", integration_type="REST", request_type="POST", input_schema={"type": "object"})] # This tool already exists - - gateway_service._initialize_gateway = AsyncMock(return_value=({"tools": {"listChanged": True}}, mock_tools, [], [])) - gateway_service._notify_gateway_added = AsyncMock() - - mock_model = Mock() - mock_model.masked.return_value = mock_model - mock_model.name = "tool_gateway" - - monkeypatch.setattr( - "mcpgateway.services.gateway_service.GatewayRead.model_validate", - lambda x: mock_model, - ) - gateway_create = GatewayCreate( name="tool_gateway", - url="http://example.com/gateway", + url="http://example.com/gateway", # Same URL as existing description="Gateway with existing tools", ) with pytest.raises(GatewayDuplicateConflictError) as exc_info: await gateway_service.register_gateway(test_db, gateway_create) + + # Verify the error details + assert exc_info.value.gateway_id == 123 + assert exc_info.value.enabled is True - err = exc_info.value - assert "Public Gateway already exists with URL" in str(err) - assert err.gateway_id == existing_tool.id - assert err.enabled is True # ──────────────────────────────────────────────────────────────────── # Validate Gateway URL - Parameterized Tests From 96807bbdf26faab7267acfc61f4681127c97fdf9 Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Wed, 12 Nov 2025 12:29:12 +0530 Subject: [PATCH 10/11] removed ununsed import Signed-off-by: Keval Mahajan --- .../f3a3a3d901b8_remove_gateway_url_unique_constraint.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py index 7e20c3c14..cc0b322f8 100644 --- a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py +++ b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py @@ -18,7 +18,6 @@ # Third-Party from alembic import op -import sqlalchemy as sa from sqlalchemy.engine import Inspector # revision identifiers, used by Alembic. From b6fd5405f3a3ea4ed45a34556e04c883fe97ef0e Mon Sep 17 00:00:00 2001 From: Keval Mahajan Date: Wed, 12 Nov 2025 12:33:52 +0530 Subject: [PATCH 11/11] updated docstring Signed-off-by: Keval Mahajan --- ...b8_remove_gateway_url_unique_constraint.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py index cc0b322f8..f190bbf47 100644 --- a/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py +++ b/mcpgateway/alembic/versions/f3a3a3d901b8_remove_gateway_url_unique_constraint.py @@ -28,7 +28,24 @@ def constraint_exists(inspector, table_name, constraint_name): - """Check if a constraint exists in the table.""" + """ + Check if a specific unique constraint exists on a given table. + + This function queries the database using the provided SQLAlchemy + inspector to determine if a constraint with the given name exists. + If the check fails due to an exception (e.g., database connectivity issues), + it conservatively assumes that the constraint exists. + + Args: + inspector (sqlalchemy.engine.reflection.Inspector): SQLAlchemy inspector + instance for database introspection. + table_name (str): Name of the table to inspect. + constraint_name (str): Name of the unique constraint to check. + + Returns: + bool: True if the constraint exists or if the check could not be performed, + False if the constraint does not exist. + """ try: unique_constraints = inspector.get_unique_constraints(table_name) return any(uc["name"] == constraint_name for uc in unique_constraints)