Skip to content

Commit 13b6092

Browse files
committed
jwt handling improvements
1 parent 9f16273 commit 13b6092

5 files changed

Lines changed: 126 additions & 122 deletions

File tree

auth/auth_info_middleware.py

Lines changed: 5 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
Authentication middleware to populate context state with user information
33
"""
44

5-
import jwt
65
import logging
76
import time
87
from fastmcp.server.middleware import Middleware, MiddlewareContext
@@ -187,90 +186,11 @@ async def _process_request_for_auth(self, context: MiddlewareContext):
187186
)
188187

189188
else:
190-
# Decode JWT to get user info
191-
logger.info("Processing JWT token")
192-
try:
193-
token_payload = jwt.decode(
194-
token_str, options={"verify_signature": False}
195-
)
196-
logger.info(
197-
f"JWT payload decoded: {list(token_payload.keys())}"
198-
)
199-
200-
# Create an AccessToken-like object
201-
access_token = WorkspaceAccessToken(
202-
token=token_str,
203-
client_id=token_payload.get("client_id", "unknown"),
204-
scopes=token_payload.get("scope", "").split()
205-
if token_payload.get("scope")
206-
else [],
207-
session_id=token_payload.get(
208-
"sid",
209-
token_payload.get(
210-
"jti",
211-
token_payload.get("session_id", "unknown"),
212-
),
213-
),
214-
expires_at=token_payload.get("exp", 0),
215-
claims=token_payload,
216-
sub=token_payload.get("sub"),
217-
email=token_payload.get("email"),
218-
)
219-
220-
# Store in context state
221-
context.fastmcp_context.set_state(
222-
"access_token", access_token
223-
)
224-
225-
# Store additional user info
226-
context.fastmcp_context.set_state(
227-
"user_id", token_payload.get("sub")
228-
)
229-
context.fastmcp_context.set_state(
230-
"username",
231-
token_payload.get(
232-
"username", token_payload.get("email")
233-
),
234-
)
235-
context.fastmcp_context.set_state(
236-
"name", token_payload.get("name")
237-
)
238-
context.fastmcp_context.set_state(
239-
"auth_time", token_payload.get("auth_time")
240-
)
241-
context.fastmcp_context.set_state(
242-
"issuer", token_payload.get("iss")
243-
)
244-
context.fastmcp_context.set_state(
245-
"audience", token_payload.get("aud")
246-
)
247-
context.fastmcp_context.set_state(
248-
"jti", token_payload.get("jti")
249-
)
250-
context.fastmcp_context.set_state(
251-
"auth_provider_type", self.auth_provider_type
252-
)
253-
254-
# Set the definitive authentication state for JWT tokens
255-
user_email = token_payload.get(
256-
"email", token_payload.get("username")
257-
)
258-
if user_email:
259-
context.fastmcp_context.set_state(
260-
"authenticated_user_email", user_email
261-
)
262-
context.fastmcp_context.set_state(
263-
"authenticated_via", "jwt_token"
264-
)
265-
authenticated_user = user_email
266-
auth_via = "jwt_token"
267-
268-
except jwt.DecodeError:
269-
logger.error("Failed to decode JWT token")
270-
except Exception as e:
271-
logger.error(
272-
f"Error processing JWT: {type(e).__name__}"
273-
)
189+
# Non-Google JWT tokens require verification
190+
# SECURITY: Never set authenticated_user_email from unverified tokens
191+
logger.debug(
192+
"Unverified JWT token rejected - only verified tokens accepted"
193+
)
274194
else:
275195
logger.debug("No Bearer token in Authorization header")
276196
else:

auth/mcp_session_middleware.py

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,8 @@ async def dispatch(self, request: Request, call_next: Callable) -> Any:
6262
mcp_session_id = request.state.session_id
6363
logger.debug(f"Found FastMCP session ID: {mcp_session_id}")
6464

65-
# Also check Authorization header for bearer tokens
66-
auth_header = headers.get("authorization")
67-
if (
68-
auth_header
69-
and auth_header.lower().startswith("bearer ")
70-
and not user_email
71-
):
72-
try:
73-
import jwt
74-
75-
token = auth_header[7:] # Remove "Bearer " prefix
76-
# Decode without verification to extract email
77-
claims = jwt.decode(token, options={"verify_signature": False})
78-
user_email = claims.get("email")
79-
if user_email:
80-
logger.debug(f"Extracted user email from JWT: {user_email}")
81-
except Exception:
82-
pass
65+
# SECURITY: Do not decode JWT without verification
66+
# User email must come from verified sources only (FastMCP auth context)
8367

8468
# Build session context
8569
if session_id or auth_context or user_email or mcp_session_id:

auth/oauth21_session_store.py

Lines changed: 105 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -153,22 +153,22 @@ def extract_session_from_headers(headers: Dict[str, str]) -> Optional[str]:
153153
# Try Authorization header for Bearer token
154154
auth_header = headers.get("authorization") or headers.get("Authorization")
155155
if auth_header and auth_header.lower().startswith("bearer "):
156-
# Extract bearer token and try to find associated session
157156
token = auth_header[7:] # Remove "Bearer " prefix
157+
# Intentionally ignore empty tokens - "Bearer " with no token should not
158+
# create a session context (avoids hash collisions on empty string)
158159
if token:
159-
# Look for a session that has this access token
160-
# This requires scanning sessions, but bearer tokens should be unique
160+
# Use thread-safe lookup to find session by access token
161161
store = get_oauth21_session_store()
162-
for user_email, session_info in store._sessions.items():
163-
if session_info.get("access_token") == token:
164-
return session_info.get("session_id") or f"bearer_{user_email}"
162+
session_id = store.find_session_id_for_access_token(token)
163+
if session_id:
164+
return session_id
165165

166-
# If no session found, create a temporary session ID from token hash
167-
# This allows header-based authentication to work with session context
168-
import hashlib
166+
# If no session found, create a temporary session ID from token hash
167+
# This allows header-based authentication to work with session context
168+
import hashlib
169169

170-
token_hash = hashlib.sha256(token.encode()).hexdigest()[:8]
171-
return f"bearer_token_{token_hash}"
170+
token_hash = hashlib.sha256(token.encode()).hexdigest()[:8]
171+
return f"bearer_token_{token_hash}"
172172

173173
return None
174174

@@ -325,6 +325,32 @@ def store_session(
325325
"""
326326
with self._lock:
327327
normalized_expiry = _normalize_expiry_to_naive_utc(expiry)
328+
329+
# Clean up previous session mappings for this user before storing new one
330+
old_session = self._sessions.get(user_email)
331+
if old_session:
332+
old_mcp_session_id = old_session.get("mcp_session_id")
333+
old_session_id = old_session.get("session_id")
334+
# Remove old MCP session mapping if it differs from new one
335+
if old_mcp_session_id and old_mcp_session_id != mcp_session_id:
336+
if old_mcp_session_id in self._mcp_session_mapping:
337+
del self._mcp_session_mapping[old_mcp_session_id]
338+
logger.debug(
339+
f"Removed stale MCP session mapping: {old_mcp_session_id}"
340+
)
341+
if old_mcp_session_id in self._session_auth_binding:
342+
del self._session_auth_binding[old_mcp_session_id]
343+
logger.debug(
344+
f"Removed stale auth binding: {old_mcp_session_id}"
345+
)
346+
# Remove old OAuth session binding if it differs from new one
347+
if old_session_id and old_session_id != session_id:
348+
if old_session_id in self._session_auth_binding:
349+
del self._session_auth_binding[old_session_id]
350+
logger.debug(
351+
f"Removed stale OAuth session binding: {old_session_id}"
352+
)
353+
328354
session_info = {
329355
"access_token": access_token,
330356
"refresh_token": refresh_token,
@@ -570,6 +596,9 @@ def remove_session(self, user_email: str):
570596
if not mcp_session_id:
571597
logger.info(f"Removed OAuth 2.1 session for {user_email}")
572598

599+
# Clean up any orphaned mappings that may have accumulated
600+
self._cleanup_orphaned_mappings_locked()
601+
573602
def has_session(self, user_email: str) -> bool:
574603
"""Check if a user has an active session."""
575604
with self._lock:
@@ -597,6 +626,71 @@ def get_stats(self) -> Dict[str, Any]:
597626
"mcp_sessions": list(self._mcp_session_mapping.keys()),
598627
}
599628

629+
def find_session_id_for_access_token(self, token: str) -> Optional[str]:
630+
"""
631+
Thread-safe lookup of session ID by access token.
632+
633+
Args:
634+
token: The access token to search for
635+
636+
Returns:
637+
Session ID if found, None otherwise
638+
"""
639+
with self._lock:
640+
for user_email, session_info in self._sessions.items():
641+
if session_info.get("access_token") == token:
642+
return session_info.get("session_id") or f"bearer_{user_email}"
643+
return None
644+
645+
def _cleanup_orphaned_mappings_locked(self) -> int:
646+
"""Remove orphaned mappings. Caller must hold lock."""
647+
# Collect valid session IDs and mcp_session_ids from active sessions
648+
valid_session_ids = set()
649+
valid_mcp_session_ids = set()
650+
for session_info in self._sessions.values():
651+
if session_info.get("session_id"):
652+
valid_session_ids.add(session_info["session_id"])
653+
if session_info.get("mcp_session_id"):
654+
valid_mcp_session_ids.add(session_info["mcp_session_id"])
655+
656+
removed = 0
657+
658+
# Remove orphaned MCP session mappings
659+
orphaned_mcp = [
660+
sid for sid in self._mcp_session_mapping
661+
if sid not in valid_mcp_session_ids
662+
]
663+
for sid in orphaned_mcp:
664+
del self._mcp_session_mapping[sid]
665+
removed += 1
666+
logger.debug(f"Removed orphaned MCP session mapping: {sid}")
667+
668+
# Remove orphaned auth bindings
669+
valid_bindings = valid_session_ids | valid_mcp_session_ids
670+
orphaned_bindings = [
671+
sid for sid in self._session_auth_binding
672+
if sid not in valid_bindings
673+
]
674+
for sid in orphaned_bindings:
675+
del self._session_auth_binding[sid]
676+
removed += 1
677+
logger.debug(f"Removed orphaned auth binding: {sid}")
678+
679+
if removed > 0:
680+
logger.info(f"Cleaned up {removed} orphaned session mappings/bindings")
681+
682+
return removed
683+
684+
def cleanup_orphaned_mappings(self) -> int:
685+
"""
686+
Remove orphaned entries from mcp_session_mapping and session_auth_binding.
687+
688+
Returns:
689+
Number of orphaned entries removed
690+
"""
691+
with self._lock:
692+
return self._cleanup_orphaned_mappings_locked()
693+
600694

601695
# Global instance
602696
_global_store = OAuth21SessionStore()

auth/oauth_config.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"""
1010

1111
import os
12+
from threading import RLock
1213
from urllib.parse import urlparse
1314
from typing import List, Optional, Dict, Any
1415

@@ -356,35 +357,40 @@ def get_authorization_server_metadata(
356357
return metadata
357358

358359

359-
# Global configuration instance
360+
# Global configuration instance with thread-safe access
360361
_oauth_config = None
362+
_oauth_config_lock = RLock()
361363

362364

363365
def get_oauth_config() -> OAuthConfig:
364366
"""
365367
Get the global OAuth configuration instance.
366368
369+
Thread-safe singleton accessor.
370+
367371
Returns:
368372
The singleton OAuth configuration instance
369373
"""
370374
global _oauth_config
371-
if _oauth_config is None:
372-
_oauth_config = OAuthConfig()
373-
return _oauth_config
375+
with _oauth_config_lock:
376+
if _oauth_config is None:
377+
_oauth_config = OAuthConfig()
378+
return _oauth_config
374379

375380

376381
def reload_oauth_config() -> OAuthConfig:
377382
"""
378383
Reload the OAuth configuration from environment variables.
379384
380-
This is useful for testing or when environment variables change.
385+
Thread-safe reload that prevents races with concurrent access.
381386
382387
Returns:
383388
The reloaded OAuth configuration instance
384389
"""
385390
global _oauth_config
386-
_oauth_config = OAuthConfig()
387-
return _oauth_config
391+
with _oauth_config_lock:
392+
_oauth_config = OAuthConfig()
393+
return _oauth_config
388394

389395

390396
# Convenience functions for backward compatibility

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)