Skip to content

Commit 7cda2f0

Browse files
authored
fix: batch 6 hardening for oauth secrets, masking, auth timing, and outbound URL validation (h-batch-6) (#3115)
* fix: oauth config hardening and behavior consistency Refs: A-02, A-05, O-10, O-17 - centralize oauth secret protection for service-layer CRUD - add server oauth masking parity for read/list responses - keep oauth secret decrypt to runtime token exchange paths - expand regression coverage for encryption and masking behavior Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: email auth timing hardening and behavior consistency Refs: A-06 - add dummy password verification on early login failures - enforce configurable minimum failed-login response duration - add focused regression tests for timing guard paths Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: outbound url validation hardening and behavior consistency Refs: S-02, S-03 - validate admin gateway test base URL before outbound requests - validate llmchat connect server URL before session setup - add regression tests for strict local/private URL rejection Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: regression coverage hardening and behavior consistency Refs: A-02, A-05, A-06, O-10, O-17 - add branch-focused regression tests for oauth secret handling and runtime decrypt guards - add legacy update-object coverage for server oauth update path - align helper docstrings with linting policy requirements Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Update tests Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: hardening consistency for oauth storage, auth timing, and SSRF validation (A-02 A-05 A-06 O-10 O-17 S-02 S-03) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: harden admin endpoints and align load-test payloads Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 5a51f10 commit 7cda2f0

32 files changed

+1619
-156
lines changed

.env.example

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,9 @@ PLUGINS_CLI_MARKUP_MODE=rich
858858
# ACCOUNT_LOCKOUT_DURATION_MINUTES=1
859859
# Send lockout notification emails when an account is locked
860860
# ACCOUNT_LOCKOUT_NOTIFICATION_ENABLED=true
861+
# Minimum response time for failed login attempts (milliseconds)
862+
# Helps reduce timing-based account enumeration
863+
# FAILED_LOGIN_MIN_RESPONSE_MS=250
861864

862865
# Self-Service Password Reset
863866
# Enable forgot-password and reset-password workflows

CHANGELOG.md

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66

77
### Overview
88

9-
This release **tightens production defaults** and adds **defense-in-depth controls** across SSRF, transports, OIDC, and authorization (S-01, O-01, O-02, O-03, O-04, O-05, O-06, O-11, O-14, O-16, U-05, C-03, C-09, C-10, C-15, EXTRA-01, C-04, C-07, C-11, C-14, C-28, C-29):
9+
This release **tightens production defaults** and adds **defense-in-depth controls** across SSRF, transports, OIDC, OAuth secret handling, authentication timing, and authorization (S-01, S-02, S-03, A-02, A-05, A-06, O-01, O-02, O-03, O-04, O-05, O-06, O-10, O-11, O-14, O-16, O-17, U-05, C-03, C-09, C-10, C-15, EXTRA-01, C-04, C-07, C-11, C-14, C-28, C-29):
1010

11-
- **🔐 Hardening Items** - SSRF strict defaults, OIDC id_token verification, WebSocket/reverse-proxy gating, cancellation authorization, OAuth DCR access control, token scoping hardening, bearer scheme consistency, MCP/RPC token-scope enforcement, MCP transport revocation checks, session ownership enforcement, resource visibility scoping, roots authorization parity
11+
- **🔐 Hardening Items** - SSRF strict defaults with endpoint-level validation, OAuth secret at-rest protection for gateway/server/A2A configs, server OAuth read masking parity, failed-login timing hardening, OIDC id_token verification, WebSocket/reverse-proxy gating, cancellation authorization, OAuth DCR access control, token scoping hardening, bearer scheme consistency, MCP/RPC token-scope enforcement, MCP transport revocation checks, session ownership enforcement, resource visibility scoping, roots authorization parity
1212
- **🧪 Testing** - Full regression coverage for hardened paths, token scope MCP/RPC coverage, and additional allow/deny regression tests for session/resource controls
1313

1414
> **Highlights**: SSRF protection now defaults to strict mode (block localhost, private networks, fail-closed DNS). WebSocket relay and reverse-proxy transports are disabled by default behind opt-in feature flags. OIDC SSO flows verify `id_token` signatures cryptographically. Cancellation, OAuth DCR, token scoping, session ownership, and resource visibility paths enforce proper authorization gates.
@@ -140,7 +140,7 @@ This release **tightens production defaults** and adds **defense-in-depth contro
140140

141141
### Fixed
142142

143-
#### **🔐 Security** (S-01, O-01, O-05, U-05, C-03, C-04, C-07, C-09, C-10, C-11, C-14, C-15, C-28, C-29, EXTRA-01)
143+
#### **🔐 Security** (S-01, S-02, S-03, A-02, A-05, A-06, O-01, O-05, O-10, O-17, U-05, C-03, C-04, C-07, C-09, C-10, C-11, C-14, C-15, C-28, C-29, EXTRA-01)
144144
* **SSRF defaults inverted to strict** - localhost, private networks blocked; DNS fail-closed by default (S-01)
145145
* **OIDC id_token now verified** - cryptographic signature validation in SSO callback (O-01)
146146
* **OAuth DCR admin gate** - non-admin users denied access to client management endpoints (O-05)
@@ -172,6 +172,12 @@ This release **tightens production defaults** and adds **defense-in-depth contro
172172
* **SSO callback session binding** - state is bound to browser session marker and callback requires matching session binding (O-14)
173173
* **OAuth authorize/status ownership checks** - gateway visibility/team/owner checks now enforced consistently on authorize/status endpoints (O-16)
174174
* **OAuth fetch-tools access hardening** - `/oauth/fetch-tools/{gateway_id}` now reuses centralized gateway access enforcement and fails closed for non-admin null-scope contexts, with targeted regression coverage (O-15)
175+
* **OAuth config secrets now protected at rest across service CRUD** - sensitive `oauth_config` fields are encrypted on gateway/server/A2A create+update paths, with backward-compatible handling for already-encrypted values (A-02, O-10, O-17)
176+
* **Server OAuth read masking parity** - server read/list schema responses now mask sensitive OAuth keys the same way as gateway/A2A responses (A-05)
177+
* **Failed-login timing hardening** - email auth now applies dummy Argon2 verification on early failures plus a configurable minimum failed-login response floor (A-06)
178+
* **Admin gateway-test SSRF validation** - `/admin/gateways/test` now validates user-supplied target URLs before outbound requests (S-02)
179+
* **LLM chat connect SSRF validation** - `/llmchat/connect` now validates user-supplied MCP server URLs before session setup (S-03)
180+
* **OAuth DCR credential persistence hardening** - DCR-populated gateway credentials are protected before persisting to `oauth_config` (A-02, O-17)
175181
* **JWT rich-token teams semantics** - `_create_jwt_token` now preserves explicit `teams=None` as JSON `null` while still allowing omitted teams claims, restoring deterministic admin-token scope behavior for fail-closed ownership checks
176182
* **Token revocation fail-open documented** - security-features and securing docs updated to reflect availability trade-off (U-05)
177183
* **Health diagnostics auth consistency** - `/health/security` now uses standard bearer JWT validation flow.
@@ -196,6 +202,10 @@ This release **tightens production defaults** and adds **defense-in-depth contro
196202
* **C-28**: Resource event subscriptions now enforce per-subscriber visibility scoping
197203
* **C-29**: MCP resource subscription creation now enforces visibility checks
198204
* **C-15**: Token scoping defaults to deny for unmapped API paths
205+
* **A-02 / O-10 / O-17**: OAuth config sensitive keys are now encrypted at service-layer persistence boundaries for gateway/server/A2A, including DCR credential writes
206+
* **A-05**: Server read/list responses now apply OAuth secret masking parity with gateway/A2A
207+
* **A-06**: Email auth failed-login paths now include dummy Argon2 verification and a configurable response-time floor
208+
* **S-02 / S-03**: Admin gateway test and LLM chat connect now validate outbound target URLs before network calls
199209
* **C-05**: JSON-RPC tool execution now requires `tools.execute` for both `tools/call` and backward-compatible direct tool method invocation
200210
* **C-18**: Get-by-ID handlers, including `GET /resources/{resource_id}/info`, now enforce scoped ownership checks in addition to middleware controls
201211
* **C-19**: All root management endpoints now require `admin.system_config`
@@ -214,6 +224,7 @@ This release **tightens production defaults** and adds **defense-in-depth contro
214224
* Updated `docker-compose.yml` with transport feature flags and local SSRF overrides
215225
* Updated Helm chart `values.yaml`, `values.schema.json`, and `README.md` with new SSRF and transport settings
216226
* Updated `docs/config.schema.json` with new settings, defaults, and `sso_generic_jwks_uri`
227+
* Added Alembic backfill migration to protect existing plaintext OAuth config secrets in gateway/server/A2A rows
217228
* Protocol version bumped to `2025-11-25` in configuration schema
218229

219230
### Documentation

docs/config.schema.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,12 @@
11371137
"title": "Account Lockout Notification Enabled",
11381138
"type": "boolean"
11391139
},
1140+
"failed_login_min_response_ms": {
1141+
"default": 250,
1142+
"description": "Minimum response duration for failed login attempts to reduce timing side channels",
1143+
"title": "Failed Login Min Response Ms",
1144+
"type": "integer"
1145+
},
11401146
"password_reset_enabled": {
11411147
"default": true,
11421148
"description": "Enable self-service password reset workflow (set false to disable public forgot/reset endpoints)",

docs/docs/config.schema.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,6 +1150,12 @@
11501150
"title": "Account Lockout Notification Enabled",
11511151
"type": "boolean"
11521152
},
1153+
"failed_login_min_response_ms": {
1154+
"default": 250,
1155+
"description": "Minimum response duration for failed login attempts to reduce timing side channels",
1156+
"title": "Failed Login Min Response Ms",
1157+
"type": "integer"
1158+
},
11531159
"password_reset_enabled": {
11541160
"default": true,
11551161
"description": "Enable self-service password reset workflow (set false to disable public forgot/reset endpoints)",

docs/docs/manage/configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,7 @@ curl -X POST -H "Authorization: Bearer $TOKEN" \
457457
| `MAX_FAILED_LOGIN_ATTEMPTS` | Maximum failed login attempts before lockout | `10` | int > 0 |
458458
| `ACCOUNT_LOCKOUT_DURATION_MINUTES` | Account lockout duration in minutes | `1` | int > 0 |
459459
| `ACCOUNT_LOCKOUT_NOTIFICATION_ENABLED` | Send lockout notification emails | `true` | bool |
460+
| `FAILED_LOGIN_MIN_RESPONSE_MS` | Minimum failed-login response duration to reduce timing side channels | `250` | int >= 0 |
460461
| `PASSWORD_RESET_ENABLED` | Enable self-service forgot-password/reset flow | `true` | bool |
461462
| `PASSWORD_RESET_TOKEN_EXPIRY_MINUTES` | Password reset token expiry window | `60` | int > 0 |
462463
| `PASSWORD_RESET_RATE_LIMIT` | Max reset requests per email in rate window | `5` | int > 0 |

docs/docs/testing/acceptance.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,9 @@ MCPGATEWAY_ADMIN_API_ENABLED=true
338338
| Admin Home | Navigate to `$GW_URL/admin` | Access admin UI | Dashboard displayed || Visual interface |
339339
| Create Tool (Form) | `curl -X POST $GW_URL/admin/tools -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" -H "Content-Type: application/x-www-form-urlencoded" -d 'name=admin_tool&url=https://api.example.com/v1/endpoint&description=Admin created tool&integrationType=REST&requestType=GET'` | Create via form | `{"message": "Tool registered successfully!", "success": true}` || Form submission |
340340
| Create Resource (Form) | `curl -X POST $GW_URL/admin/resources -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" -H "Content-Type: application/x-www-form-urlencoded" -d 'uri=admin/test&name=admin_resource&description=Created via admin&mimeType=text/plain&content=Admin content'` | Create via form | 303 redirect || Admin form endpoint |
341-
| Test Gateway Connectivity | `curl -X POST $GW_URL/admin/gateways/test -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" -H "Content-Type: application/json" -d '{"base_url": "http://localhost:8101", "path": "/health", "method": "GET", "headers": {}, "body": null}'` | Test connection | Returns status_code, latency_ms, and body || Connectivity test |
341+
| Test Gateway Connectivity | `curl -X POST $GW_URL/admin/gateways/test -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" -H "Content-Type: application/json" -d '{"base_url": "https://api.example.com", "path": "/health", "method": "GET", "headers": {}, "body": null}'` | Test connection | Returns status_code, latency_ms, and body || Connectivity test |
342+
343+
`/admin/gateways/test` enforces SSRF URL validation. Under strict defaults, localhost/private targets are blocked unless explicitly allowed (for example via `SSRF_ALLOWED_NETWORKS` or local dev overrides).
342344

343345
## 14. Input Validation Testing
344346

docs/docs/using/clients/llm-chat.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ The LLM Chat functionality is powered by the following REST API endpoints:
184184
{
185185
"user_id": "some-string",
186186
"server": {
187-
"url": "http://localhost:8000/mcp",
187+
"url": "https://mcp.example.com/mcp",
188188
"transport": "streamable_http",
189189
"auth_token": "optional-jwt-token"
190190
},
@@ -197,6 +197,8 @@ The LLM Chat functionality is powered by the following REST API endpoints:
197197
}
198198
```
199199

200+
`/llmchat/connect` validates user-supplied server URLs with SSRF protections. Under strict defaults, localhost/private addresses are rejected unless explicitly allowed (for example via `SSRF_ALLOWED_NETWORKS` or local development overrides).
201+
200202
**Response**:
201203

202204
```json

mcpgateway/admin.py

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from datetime import datetime, timedelta, timezone
2626
from functools import lru_cache, wraps
2727
import html
28+
import inspect
2829
import io
2930
import json
3031
import logging
@@ -552,6 +553,8 @@ def decorator(func_to_wrap):
552553
Returns:
553554
The wrapped function with rate limiting applied
554555
"""
556+
signature_params = inspect.signature(func_to_wrap).parameters.values()
557+
accepts_request = any(param.name == "request" or param.kind == inspect.Parameter.VAR_KEYWORD for param in signature_params)
555558

556559
@wraps(func_to_wrap)
557560
async def wrapper(*args, request: Optional[Request] = None, **kwargs):
@@ -587,8 +590,9 @@ async def wrapper(*args, request: Optional[Request] = None, **kwargs):
587590
detail=f"Rate limit exceeded. Maximum {limit} requests per minute.",
588591
)
589592
rate_limit_storage[client_ip].append(current_time)
590-
# IMPORTANT: forward request to the real endpoint
591-
return await func_to_wrap(*args, request=request, **kwargs)
593+
if accepts_request:
594+
return await func_to_wrap(*args, request=request, **kwargs)
595+
return await func_to_wrap(*args, **kwargs)
592596

593597
return wrapper
594598

@@ -12511,15 +12515,22 @@ async def admin_test_gateway(
1251112515
>>> admin_test_gateway.__name__
1251212516
'admin_test_gateway'
1251312517
"""
12514-
full_url = str(request.base_url).rstrip("/") + "/" + request.path.lstrip("/")
12515-
full_url = full_url.rstrip("/")
12516-
LOGGER.debug(f"User {get_user_email(user)} testing server at {request.base_url}.")
1251712518
start_time: float = time.monotonic()
12519+
try:
12520+
validated_base_url = SecurityValidator.validate_url(str(request.base_url), "Gateway test URL")
12521+
except ValueError as e:
12522+
LOGGER.warning("Gateway test URL validation failed for %s: %s", request.base_url, e)
12523+
latency_ms = int((time.monotonic() - start_time) * 1000)
12524+
return GatewayTestResponse(status_code=400, latency_ms=latency_ms, body={"error": "Invalid gateway URL"})
12525+
12526+
full_url = validated_base_url.rstrip("/") + "/" + request.path.lstrip("/")
12527+
full_url = full_url.rstrip("/")
12528+
LOGGER.debug(f"User {get_user_email(user)} testing server at {validated_base_url}.")
1251812529
headers = request.headers or {}
1251912530

1252012531
# Attempt to find a registered gateway matching this URL and team
1252112532
try:
12522-
gateway = gateway_service.get_first_gateway_by_url(db, str(request.base_url), team_id=team_id)
12533+
gateway = gateway_service.get_first_gateway_by_url(db, validated_base_url, team_id=team_id)
1252312534
except Exception:
1252412535
gateway = None
1252512536

@@ -13770,6 +13781,8 @@ async def admin_import_preview(request: Request, db: Session = Depends(get_db),
1377013781
except ImportValidationError as e:
1377113782
LOGGER.error(f"Import validation failed for user {user}: {str(e)}")
1377213783
raise HTTPException(status_code=400, detail=f"Invalid import data: {str(e)}")
13784+
except HTTPException:
13785+
raise
1377313786
except Exception as e:
1377413787
LOGGER.error(f"Import preview failed for user {user}: {str(e)}")
1377513788
raise HTTPException(status_code=500, detail=f"Preview failed: {str(e)}")
@@ -13834,6 +13847,8 @@ async def admin_import_configuration(request: Request, db: Session = Depends(get
1383413847
except ImportServiceError as e:
1383513848
LOGGER.error(f"Admin import failed for user {user}: {str(e)}")
1383613849
raise HTTPException(status_code=400, detail=str(e))
13850+
except HTTPException:
13851+
raise
1383713852
except Exception as e:
1383813853
LOGGER.error(f"Unexpected admin import error for user {user}: {str(e)}")
1383913854
raise HTTPException(status_code=500, detail=f"Import failed: {str(e)}")
@@ -14918,7 +14933,11 @@ async def get_resources_section(
1491814933
LOGGER.debug(f"User {user_email} requesting resources section with team_id={team_id}")
1491914934

1492014935
# Get all resources and filter by team
14921-
resources_list = await local_resource_service.list_resources(db, include_inactive=True)
14936+
resources_result = await local_resource_service.list_resources(db, include_inactive=True)
14937+
if isinstance(resources_result, tuple):
14938+
resources_list = resources_result[0]
14939+
else:
14940+
resources_list = resources_result
1492214941

1492314942
# Apply team filtering if specified
1492414943
if team_id:
@@ -14973,7 +14992,11 @@ async def get_prompts_section(
1497314992
LOGGER.debug(f"User {user_email} requesting prompts section with team_id={team_id}")
1497414993

1497514994
# Get all prompts and filter by team
14976-
prompts_list = await local_prompt_service.list_prompts(db, include_inactive=True)
14995+
prompts_result = await local_prompt_service.list_prompts(db, include_inactive=True)
14996+
if isinstance(prompts_result, tuple):
14997+
prompts_list = prompts_result[0]
14998+
else:
14999+
prompts_list = prompts_result
1497715000

1497815001
# Apply team filtering if specified
1497915002
if team_id:
@@ -15031,7 +15054,11 @@ async def get_servers_section(
1503115054
LOGGER.debug(f"User {user_email} requesting servers section with team_id={team_id}, include_inactive={include_inactive}")
1503215055

1503315056
# Get servers with optional include_inactive parameter
15034-
servers_list = await local_server_service.list_servers(db, include_inactive=include_inactive)
15057+
servers_result = await local_server_service.list_servers(db, include_inactive=include_inactive)
15058+
if isinstance(servers_result, tuple):
15059+
servers_list = servers_result[0]
15060+
else:
15061+
servers_list = servers_result
1503515062

1503615063
# Apply team filtering if specified
1503715064
if team_id:

0 commit comments

Comments
 (0)