feat(api): add POST /v1/mcp-servers/test endpoint for React UI connection testing

[Lang-Akshay]

Closes #5326

The React UI cannot reach `/admin/**` endpoints, which are reserved for the legacy HTMX admin interface. This PR exposes a dedicated v1 REST endpoint so the React "Test Connection" dialog (#5040) can test MCP server / gateway connectivity without touching admin routes.

Rather than duplicating the gateway-test handler, the core logic is extracted into a standalone `test_gateway_connectivity()` function in `gateway_service.py`. Both the existing `/admin/gateways/test` endpoint and the new `/v1/mcp-servers/test` endpoint become thin delegation wrappers around this single source of truth.

Changes

New files

• `mcpgateway/routers/mcp_servers_router.py` — `APIRouter(prefix="/v1/mcp-servers")` with `POST /test`; enforces `gateways.read` permission via `@require_permission("gateways.read", allow_admin_bypass=False)`; delegates to `test_gateway_connectivity()`
• `tests/unit/mcpgateway/routers/test_mcp_servers_router.py` — unit tests covering success, SSRF blocked, 502 on request error, non-JSON response, registered-only allowlist mode, POST with body, timeout, and `_validated_team_id` edge cases, plus deny-path regression tests (401 unauthenticated, 403 insufficient permission, 403 cross-team `team_id`, admin-bypass allow case)

Modified files

• `mcpgateway/services/gateway_service.py` — extracted `async def test_gateway_connectivity(request, team_id, user, db)` from `admin_test_gateway`; all security behaviour preserved: DNS-pinning, SSRF allowlist enforcement, OAuth (authorization code + client credentials), basic/bearer/header auth injection, and structured logging on success and failure
• `mcpgateway/admin.py` — `admin_test_gateway` body replaced with a single call to `test_gateway_connectivity()`; signature, decorators, and docstring unchanged
• `mcpgateway/middleware/token_scoping.py` — added `(POST, re.compile(r"^/mcp-servers/test(?:$|/)"), Permissions.GATEWAYS_READ)` to the route permission patterns
• `mcpgateway/main.py` — registered `mcp_servers_router` via `app.include_router()`
• `tests/unit/mcpgateway/test_admin.py` / `tests/e2e/test_admin_apis.py` — updated mock patch targets from `mcpgateway.admin.` to `mcpgateway.services.gateway_service.` to reflect the extraction

Security hardening (beyond straight extraction)

• Cross-team `team_id` rejection on both routes — both the v1 endpoint and `/admin/gateways/test` now validate the caller-supplied `team_id` against the token's `token_teams` scope: non-admin callers requesting a team outside their scope get 403 (fail-closed for public-only tokens). Prevents enumerating other teams' registered gateway hostnames via the SSRF allowlist. Admin bypass (`token_teams=None`) is preserved. Previously only the v1 route had this guard; the pre-existing admin route was exposed to the same cross-team probing risk, which this PR now closes.
• OAuth client-credentials failure now fails closed on both routes — the original admin handler set an error body on token-fetch failure but fell through and sent the outbound request without the `Authorization` header (the error body was then overwritten by the upstream response). The extracted implementation returns 502 with a clear token-retrieval error instead, matching the early-return behaviour of the authorization-code branch. This is a behaviour change to the existing `/admin/gateways/test` endpoint: callers that previously relied on the (arguably broken) unauthenticated fallback will now receive a 502 on OAuth token-fetch failure.
• `team_id` normalization parity — `_validated_team_id` treats an empty `?team_id=` as "no filter" (returns `None`), matching the admin route's `_normalize_team_id`; malformed UUIDs still return 400.

Minor cleanups

• `mcpgateway/services/metrics.py`, `mcpgateway/auth_context.py`, and the `e28cd485ad3c` migration — formatting/import-order lint fixes picked up along the way (no behaviour change)

[marekdano]

Solid, well-scoped refactor. SSRF and RBAC protections are genuinely preserved. Two items should be addressed before merge (deny-path tests; a carried-over OAuth failure path); the rest are minor.

Findings

1. gateway_service.py — OAuth client-credentials token-fetch failure silently proceeds unauthenticated (carried over from original)

In the client-credentials branch, if oauth_manager.get_access_token() raises, the except sets response_body = {"error": ...} but does not return. Execution falls through, the outbound request is sent without the Authorization header, and that response_body is immediately overwritten by response.json().

• Failure scenario: A gateway with auth_type="oauth" / client-credentials whose token endpoint is down → the caller gets whatever the unauthenticated upstream returns (often a misleading 200/401 from the target) instead of a clear token-retrieval error.
• Contrast the authorization-code branch just above, which correctly returns a 500.
• Pre-existing behavior copied verbatim — not introduced here — but since the code is freshly landing in gateway_service.py, it's a good moment to add the missing early return (or drop the dead assignment).

2. test_mcp_servers_router.py — no deny-path regression tests (violates a stated security invariant)

"Security-sensitive changes must include deny-path regression tests (unauthenticated, wrong team, insufficient permissions, feature disabled)."

• The new test file applies an autouse rbac_bypass fixture to every test, and there is not a single assertion that an unauthenticated or insufficient-permission caller is rejected (grep for 401/403/insufficient/PermissionError → nothing).
• SSRF-blocked (400) is well covered, but the RBAC enforcement this PR adds is never actually exercised - the only test that could catch a regression bypasses the mechanism under test.
• Ask: add at least one 403 case that does not apply the bypass.

3. mcp_servers_router.py:406 — empty-team_id behavior diverges from the admin route (minor)

_validated_team_id guards with if team_id is None, so ?team_id= (empty string) reaches uuid.UUID("") and raises HTTP 400. The admin route's _normalize_team_id uses if not team_id: return None, treating empty string as "no filter." Same query string, different behavior across the two endpoints. The new test locks in the stricter behavior — arguably fine, but worth a deliberate decision given the PR's parity goal.

4. Client-supplied team_id is used unverified (low / pre-existing parity)

team_id comes from an unauthenticated query param and filters the gateway/allowlist query without verifying the caller's team membership. In gateway_test_allow_registered_only mode a caller could widen the test-allowlist using another team's registered gateway URLs. This mirrors the existing admin endpoint exactly, so it's not a regression — but since it's a fresh public v1 surface, confirm that team_id here is only ever a visibility filter, never an authorization decision (per the "never trust client-provided team_id" invariant).

[Lang-Akshay]

Thanks for the review @marekdano . I have addressed the valid finding.

#	Finding	Verdict	Explanation
1	gateway_service.py — OAuth client-credentials token-fetch failure silently proceeds unauthenticated (sets response_body = {"error": ...} without return)	❌Not valid (stale)	Bug existed in oldadmin.py:14528 on main, but the shared impl this PR lands already fixes it: gateway_service.py:6767-6770 returns early with GatewayTestResponse(status_code=502, ...). Admin route now delegates to the same function (admin.py:14399), so both surfaces are fixed. Reviewer saw a pre-fix revision.
2	test_mcp_servers_router.py — no deny-path regression tests; autouse rbac_bypass neuters RBAC coverage	❌Not valid (stale)	Deny-path section exists at HEAD (test_mcp_servers_router.py:462-539): 401 unauthenticated, 403 insufficient permission, 403 cross-team team_id, plus admin-bypass allow case. Added in commit 00af47e25. The rbac_bypass fixture is a no-op for this router anyway — the decorator is baked in at import time and the fixture patches afterward — so the deny tests exercise the real require_permission decorator.
3	mcp_servers_router.py:55 — empty-string ?team_id= returns 400, diverges from admin route which treats it as "no filter"	✅Valid (minor, in scope)	Confirmed: new route guardsif team_id is None → "" hits uuid.UUID("") → 400. Admin's _normalize_team_id (admin.py:812) uses if not team_id: return None → empty string = no filter. Same query string, different behavior across the two parity endpoints; test at line 222 locks in the stricter behavior. Needs a deliberate choice: keep the stricter 400, or match admin for parity (one-line fix).
4	Client-suppliedteam_id used unverified — could widen test-allowlist with another team's gateway URLs	❌Not valid at HEAD	Three claims fail: (1) route is authenticated (get_current_user_with_permissions + @require_permission("gateways.read", allow_admin_bypass=False)); (2) membership IS verified — mcp_servers_router.py:95-98 returns 403 when a non-admin's token_teams doesn't contain the supplied team_id, fail-closed for public-only []; (3) the "widening" premise is wrong — omitting team_id already builds the allowlist from ALL enabled gateways (gateway_service.py:6644-6646), so team_id only narrows. It's a visibility filter, never an authz grant.

[marekdano]

The PR looks good!

LGTM 🚀

[gandhipratik203]

Nice cleanup, nothing broke in the SSRF/OAuth handling, and the deny-path tests hold up. LGTM.

[msureshkumar88]

E2E Verification

I ran this against a real, running instance of the gateway — not mocks — using a new script, scripts/verify-pr-5443-e2e.sh, added for this review (real uv run uvicorn, real SQLite DB, real JWTs via create_jwt_token, real HTTP via curl, one real outbound network call to https://example.com).

Config: AUTH_REQUIRED=true, MCPGATEWAY_ADMIN_API_ENABLED=true, GATEWAY_TEST_ALLOW_REGISTERED_ONLY=false + GATEWAY_TEST_ALLOWED_HOSTS=["example.com","*.example.com"], GATEWAY_ASYNC_LIFECYCLE_ENABLED=true (to register an OAuth-configured gateway without needing a live MCP handshake), FEDERATION_TIMEOUT=8. Branch fix/issue5326 @ 1943aafc5.

Note: SecurityValidator.validate_gateway_test_url() unconditionally blocks loopback/private/link-local resolved IPs for this endpoint regardless of SSRF_ALLOW_LOCALHOST/SSRF_ALLOW_PRIVATE_NETWORKS (pre-existing, not part of this diff), so a locally-run mock MCP server can't be used as the connectivity target — https://example.com was used instead for the outbound-request checks.

Results (all real HTTP round-trips):

#	Check	Result
1	POST /v1/mcp-servers/test happy path (admin token, real GET to example.com)	PASS — HTTP 200, statusCode:200
2	POST /admin/gateways/test same body/token — parity with #1	PASS — identical HTTP 200 / statusCode:200
3	No Authorization header at all	PASS — 403 CSRF_TOKEN_INVALID (pre-existing CSRF middleware fires before auth for header-less POSTs; unrelated to this PR)
4	Garbage bearer token	PASS — 401
5	Valid token scoped to tools.read only (no gateways.read)	PASS — 403 on both /v1/mcp-servers/test and /admin/gateways/test
6	Cross-team team_id — non-admin user, real member of team A only, holding a global-scope role granting gateways.read (so Layer 2/RBAC would otherwise allow any team), token minted with teams:[team A], request team_id=team B	v1: 403 {"detail":"Access to requested team is not permitted"} — admin: 200, normal successful response, for the identical request
7	SSRF: base_url=http://169.254.169.254/...	PASS — blocked with statusCode:400 on both v1 and admin
8	Closed/unreachable port (https://example.com:9999)	PASS — clean statusCode:502, no 500, no hang
9	OAuth client-credentials fails (registered gateway, token_url=http://127.0.0.1:1/token, connection refused)	PASS — /v1/mcp-servers/test → statusCode:502, {"error":"OAuth token retrieval failed for gateway"}; /admin/gateways/test → also statusCode:502 on the identical target

Also ran the existing suites, all green: pytest tests/unit/mcpgateway/routers/test_mcp_servers_router.py (17 passed), pytest tests/unit/mcpgateway/test_admin.py -k gateway_test (10 passed), pytest tests/e2e/test_admin_apis.py -k gateway_test (2 passed).

Live-confirmed both of the behavior claims called out in the PR description:

• OAuth fails closed: verified end-to-end that the outbound HTTP client is never invoked when OAuthManager.get_access_token() raises (log shows 3 failed token attempts, then a clean 502, with no httpx request log line at all — vs. the happy-path run, which does show an httpx request line). This is correctly a shared fix — it also changes /admin/gateways/test's existing behavior (see Blocking finding Open Source release of MCP Context Forge - MCP Gateway #2 below on how to characterize that).
• Cross-team team_id rejection is v1-only: reproduced live with a setup where Layer-2 RBAC would otherwise grant the request (global-scope role) so the only thing separating the two outcomes is the explicit token_teams check in the new router. /admin/gateways/test returns the connectivity result instead of a 403 for the same cross-team request.

---

Code Review

Scope: only files touched by git diff main...HEAD per the PR.

Blocking

1. CWE-863 — cross-team hardening is incomplete: /admin/gateways/test still allows team-scope probing (mcpgateway/routers/mcp_servers_router.py:95-101 vs mcpgateway/admin.py:14377-14399). The new router's own comment explains the risk precisely: "A caller-supplied team_id outside that list would allow enumerating other teams' registered gateway hostnames (SSRF allowlist)." That's exactly what I reproduced live in E2E check Update testing docs, copilot docs and minor fix to servers/ also offering a servers (no /) endpoint #6 above against /admin/gateways/test. Since test_gateway_connectivity() is now explicitly the shared implementation for both routes, it'd be worth moving this check into the shared function (or adding the same guard to admin_test_gateway) rather than leaving the pre-existing admin route exposed to the risk this PR otherwise fixes for the new route. Worth discussing whether this is an intentional scope decision (e.g., admin routes assumed to be behind a different trust boundary) — if so, it'd be good to document that explicitly, since right now it reads as an oversight given how deliberately the v1-only check was written.

2. Undocumented behavior change to the existing /admin/gateways/test endpoint. Because admin_test_gateway now delegates entirely to the shared test_gateway_connectivity() (mcpgateway/admin.py:14399), the OAuth-fails-closed fix (mcpgateway/services/gateway_service.py:6807-6810) also changes behavior for existing admin callers — previously an OAuth token-fetch failure logged an error but fell through and sent an unauthenticated request anyway (confirmed by reading the pre-PR code in the diff). That's a legitimate bugfix, but it's a behavior change to an existing, not just a new, endpoint, and neither the commit message ("Refactor admin_test_gateway to delegate to the shared function" — phrased as a pure refactor) nor the PR description calls this out. Worth a line in the PR description clarifying that /admin/gateways/test callers relying on the old (arguably broken) fallback-to-unauthenticated behavior will now get a 502 instead.

Suggestions

• Duplicate team_id validators. mcp_servers_router._validated_team_id() (mcpgateway/routers/mcp_servers_router.py:41-70) and admin._normalize_team_id()/_validated_team_id_param() (mcpgateway/admin.py:800-835) implement identical normalize-or-400 logic independently. Since the PR is already unifying the gateway-test logic into a shared function, this would be a natural candidate to unify too (e.g., move into a shared utility) so the two can't silently drift.
• Fragile exact-string URL match undermines the OAuth fix in practice. test_gateway_connectivity()'s gateway lookup (mcpgateway/services/gateway_service.py:6760, DbGateway.url == validated_base_url) is unchanged from the old admin code, but it's now reachable from a public non-admin route accepting arbitrary caller input. GatewayTestRequest.base_url is typed AnyHttpUrl, which pydantic normalizes to append a trailing slash for bare-domain URLs, while GatewayCreate.url is a plain str stored verbatim (no such normalization) — so a gateway registered as https://api.example.com (no trailing slash, the common case) will silently fail this exact-match lookup when tested with base_url: "https://api.example.com", because by the time it reaches this comparison it's been coerced to "https://api.example.com/". When the lookup misses, gateway is None and the whole OAuth/auth-header injection branch is skipped — the request goes out unauthenticated with a normal 200, no error, no indication to the caller that auth wasn't applied. I reproduced this live in my first test run (had to explicitly register with a trailing slash to exercise the OAuth path at all). This is pre-existing, unchanged code, so it's out of strict diff scope, but this PR is what turns it into a public-surface concern — worth a follow-up to normalize both sides before comparing (e.g., strip trailing slash, or compare via urlparse equality).
• No log line on cross-team rejection. The SSRF-validation-failure path logs via logger.warning(...) (mcpgateway/services/gateway_service.py:96-102), but the explicit cross-team check in mcp_servers_router.py:95-101 just raises HTTPException with no log call. For incident forensics it'd be useful to log the rejected team_id/caller the same way the SSRF path does.
• Test coverage doesn't actually pin the OAuth-fails-closed fix. test_admin_test_gateway_oauth_client_credentials_token_error (tests/unit/mcpgateway/test_admin.py:15930-15957) asserts status_code == 502, but its MockClient.request() raises httpx.RequestError unconditionally — so the same assertion would also pass under the old, buggy fall-through behavior (falls through, calls the client, which also raises, also nets a 502). It doesn't distinguish "failed closed before any request" from "fell through and the outbound call also happened to fail." Given how central this behavior is to the PR, it'd be worth asserting the mock client was never called (I verified the real fail-closed behavior independently via the E2E run's logs, which show no httpx request line for this case).
• No CHANGELOG/docs entry for the new public POST /v1/mcp-servers/test surface — worth adding given it's a new externally-callable API, not just an internal refactor.

Other review notes (no action needed)

• Verified mcpgateway/services/metrics.py, mcpgateway/auth_context.py, and the alembic migration diff are genuinely import-order/formatting only (isort reordering, ruff format's string-join collapse) — no substantive change. Ignored .secrets.baseline per convention.
• Issue reference: Closes #5326 is present and correct.
• Scope is appropriately tight — the extraction touches exactly the files needed, no unrelated feature creep.
• Ordering of checks in the v1 router (auth → RBAC permission → cross-team check → SSRF validation → outbound request) is correct and fails fast without leaking allowlist/topology details in error messages (generic messages throughout — no CWE-200 concern found).

---

Verdict

E2E passed across all 9 real-HTTP checks plus the full existing test suite, and this PR's two headline claims (OAuth fails closed, v1 cross-team hardening) are verified live and accurate as far as they go. But E2E check #6 also live-confirms a real, reachable authorization gap: the same cross-team probing risk this PR fixes for the new route is left open on /admin/gateways/test, which shares the exact same underlying function. Requesting changes to close that gap (or get an explicit call on why it's out of scope) before merge; happy to re-review quickly once addressed.

[msureshkumar88]

E2E-verified this live end-to-end (real server, real HTTP, real JWTs) — the two headline claims (OAuth fails closed, v1 cross-team hardening) both check out. But the same live testing also confirms /admin/gateways/test still lacks the cross-team team_id check the new v1 route adds, even though both now share the identical underlying test_gateway_connectivity() function — a non-admin token scoped to team A can use the admin route to test connectivity against team B's registered-gateway allowlist, which is exactly the enumeration risk the new router's own code comment calls out. Requesting changes to close that gap (or get an explicit decision that it's out of scope) before merge. Full details, plus a few non-blocking suggestions, in my comment above.

[Lang-Akshay]

Thanks for the review @msureshkumar88
Have addressed the two blocking issue

• Finding 1 fix — admin.py:14399: added identical cross-team guard before test_gateway_connectivity() call. Scoped callers supplying team_id outside their token_teams now get 403 on the admin route, same as the v1 route.

• Finding 2 — PR description already documented the OAuth behavior change in "Security hardening." Updated the section to explicitly flag it as a behavior change to the existing /admin/gateways/test endpoint and strengthened the cross-team guard note to cover both routes.