Skip to content

Commit 4a74b09

Browse files
committed
fix(auth): only hard-deny revocation/disabled 401s, not all auth failures
The auth middleware was hard-denying ALL 401/403 HTTPExceptions, which broke registration scripts and other callers using minimal JWT claims (no user/teams/token_use). These previously fell through to route-level auth handlers. Narrow the hard-deny to only security-critical rejections: "Token has been revoked", "Account disabled", and "Token validation failed". All other 401/403s continue as anonymous for route-level handling. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 596b094 commit 4a74b09

File tree

2 files changed

+35
-8
lines changed

2 files changed

+35
-8
lines changed

mcpgateway/middleware/auth_middleware.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,13 @@ async def dispatch(self, request: Request, call_next: Callable) -> Response:
146146
logger.debug(f"Failed to close database session: {close_error}")
147147

148148
except HTTPException as e:
149-
if e.status_code in (401, 403):
150-
# Security-critical rejections (revoked tokens, disabled accounts) must NOT
151-
# be swallowed — propagate as a hard deny so revoked/expired tokens cannot
152-
# fall through to public-access endpoints.
149+
# Only hard-deny for security-critical rejections (revoked tokens,
150+
# disabled accounts). Other 401s (e.g. malformed tokens, missing
151+
# claims) fall through so route-level auth can handle them — this
152+
# preserves backwards compatibility with registration scripts and
153+
# other callers that use minimal JWT claims.
154+
_HARD_DENY_DETAILS = frozenset({"Token has been revoked", "Account disabled", "Token validation failed"})
155+
if e.status_code in (401, 403) and e.detail in _HARD_DENY_DETAILS:
153156
logger.info(f"✗ Auth rejected ({e.status_code}): {e.detail}")
154157

155158
if log_failure:

tests/unit/mcpgateway/middleware/test_auth_middleware.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ async def test_http_403_returns_json_deny_for_api_request():
344344

345345
with patch("mcpgateway.middleware.auth_middleware._should_log_auth_success", return_value=False), \
346346
patch("mcpgateway.middleware.auth_middleware._should_log_auth_failure", return_value=False), \
347-
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=403, detail="Forbidden"))):
347+
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=403, detail="Account disabled"))):
348348
response = await middleware.dispatch(request, call_next)
349349

350350
assert response.status_code == 403
@@ -427,7 +427,7 @@ async def test_http_401_logging_db_error_handled():
427427
patch("mcpgateway.middleware.auth_middleware._should_log_auth_failure", return_value=True), \
428428
patch("mcpgateway.middleware.auth_middleware.SessionLocal", return_value=mock_db), \
429429
patch("mcpgateway.middleware.auth_middleware.security_logger", mock_security_logger), \
430-
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Revoked"))):
430+
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Token has been revoked"))):
431431
response = await middleware.dispatch(request, call_next)
432432

433433
# Should still return 401 despite logging failure
@@ -457,7 +457,7 @@ async def test_http_401_logging_db_close_error_handled():
457457
patch("mcpgateway.middleware.auth_middleware._should_log_auth_failure", return_value=True), \
458458
patch("mcpgateway.middleware.auth_middleware.SessionLocal", return_value=mock_db), \
459459
patch("mcpgateway.middleware.auth_middleware.security_logger", mock_security_logger), \
460-
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Revoked"))):
460+
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Token has been revoked"))):
461461
response = await middleware.dispatch(request, call_next)
462462

463463
assert response.status_code == 401
@@ -487,6 +487,30 @@ async def test_non_401_403_http_exception_continues_as_anonymous():
487487
assert response.status_code == 200
488488

489489

490+
@pytest.mark.asyncio
491+
async def test_non_revocation_401_falls_through_as_anonymous():
492+
"""Non-revocation 401 (e.g. malformed token) continues as anonymous for route-level auth."""
493+
from fastapi import HTTPException
494+
495+
middleware = AuthContextMiddleware(app=AsyncMock())
496+
call_next = AsyncMock(return_value=Response("ok"))
497+
request = MagicMock(spec=Request)
498+
request.url.path = "/api/tools"
499+
request.cookies = {"jwt_token": "minimal_jwt"}
500+
request.headers = {"accept": "application/json"}
501+
request.client = MagicMock()
502+
request.client.host = "127.0.0.1"
503+
504+
with patch("mcpgateway.middleware.auth_middleware._should_log_auth_success", return_value=False), \
505+
patch("mcpgateway.middleware.auth_middleware._should_log_auth_failure", return_value=False), \
506+
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Invalid authentication credentials"))):
507+
response = await middleware.dispatch(request, call_next)
508+
509+
# Non-revocation 401 should fall through, not hard-deny
510+
call_next.assert_awaited_once_with(request)
511+
assert response.status_code == 200
512+
513+
490514
@pytest.mark.asyncio
491515
async def test_http_401_referer_admin_continues_for_redirect():
492516
"""HTTPException 401 with Referer: /admin continues for RBAC redirect (not JSON deny)."""
@@ -527,7 +551,7 @@ async def test_http_401_json_deny_includes_security_headers():
527551

528552
with patch("mcpgateway.middleware.auth_middleware._should_log_auth_success", return_value=False), \
529553
patch("mcpgateway.middleware.auth_middleware._should_log_auth_failure", return_value=False), \
530-
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Revoked"))):
554+
patch("mcpgateway.middleware.auth_middleware.get_current_user", AsyncMock(side_effect=HTTPException(status_code=401, detail="Token has been revoked"))):
531555
response = await middleware.dispatch(request, call_next)
532556

533557
assert response.status_code == 401

0 commit comments

Comments
 (0)