Skip to content

Commit f3d2f53

Browse files
committed
### Code Review Fixes
**Round 1 - Initial Review:** - ✅ Added ID-JAG expiry tracking to prevent stale tokens - ✅ Enhanced audience override with WARNING level logging - ✅ Documented error handling delegation pattern - ✅ Added metadata stability documentation - ✅ Fixed empty scope handling with strip() check - ✅ Added negative JWT decode tests **Round 2 - Systematic Review:** - ✅ Made ID-JAG expiry configurable (DEFAULT_ID_JAG_EXPIRY_SECONDS = 900) - ✅ Added comprehensive thread safety documentation. - ✅ Moved time import to module level - ✅ Extracted magic numbers to constants - ✅ Enhanced logging with default expiry notifications **Round 3 - Final Fixes:** - ✅ Changed validation exceptions to OAuthFlowError for consistency - ✅ Added edge case test for oauth_metadata=None - ✅ Fixed critical bug: instance variable vs class constant ### Test Coverage **Statistics:** - 43 test cases covering all code paths - 100% code coverage (166 statements, 44 branches) - Edge cases: expired tokens, malformed JWT, HTTP errors, empty scope - Conformance test integration: `auth/cross-app-access-complete-flow` ### Breaking Changes **None.** All changes are backward compatible with optional parameters.
1 parent e2f3136 commit f3d2f53

File tree

5 files changed

+812
-84
lines changed

5 files changed

+812
-84
lines changed

.github/actions/conformance/client.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,9 @@ async def run_cross_app_access_complete_flow(server_url: str) -> None:
414414

415415
async def _run_auth_session(server_url: str, oauth_auth: OAuthClientProvider) -> None:
416416
"""Common session logic for all OAuth flows."""
417-
client = httpx.AsyncClient(auth=oauth_auth, timeout=30.0)
417+
# Allow timeout to be configured via environment variable for different test scenarios
418+
timeout = float(os.environ.get("MCP_CONFORMANCE_TIMEOUT", "30.0"))
419+
client = httpx.AsyncClient(auth=oauth_auth, timeout=timeout)
418420
async with streamable_http_client(url=server_url, http_client=client) as (read_stream, write_stream):
419421
async with ClientSession(
420422
read_stream, write_stream, elicitation_callback=default_elicitation_callback

README.v2.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2630,6 +2630,8 @@ async def advanced_manual_flow() -> None:
26302630
async with httpx.AsyncClient() as client:
26312631
# Step 1: Exchange ID token for ID-JAG
26322632
id_jag = await enterprise_auth.exchange_token_for_id_jag(client)
2633+
# WARNING: Only log tokens in development/testing environments
2634+
# In production, NEVER log tokens or token fragments as they are sensitive credentials
26332635
print(f"Obtained ID-JAG: {id_jag[:50]}...")
26342636

26352637
# Step 2: Build JWT bearer grant request
@@ -2646,13 +2648,54 @@ async def advanced_manual_flow() -> None:
26462648
token_type=token_data["token_type"],
26472649
expires_in=token_data.get("expires_in"),
26482650
)
2651+
# WARNING: In production, do not log token expiry or any token information
26492652
print(f"Access token obtained, expires in: {access_token.expires_in}s")
26502653

26512654
# Use the access token for API calls
26522655
_ = {"Authorization": f"Bearer {access_token.access_token}"}
26532656
# ... make authenticated requests with headers
26542657

26552658

2659+
async def token_refresh_example() -> None:
2660+
"""Example showing how to refresh tokens when they expire.
2661+
2662+
When your access token expires, you need to obtain a fresh ID token
2663+
from your enterprise IdP and use the refresh helper method.
2664+
"""
2665+
# Initial setup
2666+
id_token = await get_id_token_from_idp()
2667+
2668+
token_exchange_params = TokenExchangeParameters.from_id_token(
2669+
id_token=id_token,
2670+
mcp_server_auth_issuer="https://auth.mcp-server.example.com",
2671+
mcp_server_resource_id="https://mcp-server.example.com",
2672+
)
2673+
2674+
enterprise_auth = EnterpriseAuthOAuthClientProvider(
2675+
server_url="https://mcp-server.example.com",
2676+
client_metadata=OAuthClientMetadata(
2677+
redirect_uris=[AnyUrl("http://localhost:3000/callback")],
2678+
),
2679+
storage=SimpleTokenStorage(),
2680+
idp_token_endpoint="https://your-idp.com/oauth2/v1/token",
2681+
token_exchange_params=token_exchange_params,
2682+
)
2683+
2684+
_ = httpx.AsyncClient(auth=enterprise_auth, timeout=30.0)
2685+
2686+
# Use the client for MCP operations...
2687+
# ... time passes and token expires ...
2688+
2689+
# When token expires, get a fresh ID token from your IdP
2690+
new_id_token = await get_id_token_from_idp()
2691+
2692+
# Refresh the authentication using the helper method
2693+
await enterprise_auth.refresh_with_new_id_token(new_id_token)
2694+
2695+
# Next API call will automatically use the refreshed tokens
2696+
# No need to recreate the client or session
2697+
2698+
26562699
if __name__ == "__main__":
26572700
asyncio.run(main())
26582701
```

examples/snippets/clients/enterprise_managed_auth_client.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,8 @@ async def advanced_manual_flow() -> None:
188188
async with httpx.AsyncClient() as client:
189189
# Step 1: Exchange ID token for ID-JAG
190190
id_jag = await enterprise_auth.exchange_token_for_id_jag(client)
191+
# WARNING: Only log tokens in development/testing environments
192+
# In production, NEVER log tokens or token fragments as they are sensitive credentials
191193
print(f"Obtained ID-JAG: {id_jag[:50]}...")
192194

193195
# Step 2: Build JWT bearer grant request
@@ -204,12 +206,53 @@ async def advanced_manual_flow() -> None:
204206
token_type=token_data["token_type"],
205207
expires_in=token_data.get("expires_in"),
206208
)
209+
# WARNING: In production, do not log token expiry or any token information
207210
print(f"Access token obtained, expires in: {access_token.expires_in}s")
208211

209212
# Use the access token for API calls
210213
_ = {"Authorization": f"Bearer {access_token.access_token}"}
211214
# ... make authenticated requests with headers
212215

213216

217+
async def token_refresh_example() -> None:
218+
"""Example showing how to refresh tokens when they expire.
219+
220+
When your access token expires, you need to obtain a fresh ID token
221+
from your enterprise IdP and use the refresh helper method.
222+
"""
223+
# Initial setup
224+
id_token = await get_id_token_from_idp()
225+
226+
token_exchange_params = TokenExchangeParameters.from_id_token(
227+
id_token=id_token,
228+
mcp_server_auth_issuer="https://auth.mcp-server.example.com",
229+
mcp_server_resource_id="https://mcp-server.example.com",
230+
)
231+
232+
enterprise_auth = EnterpriseAuthOAuthClientProvider(
233+
server_url="https://mcp-server.example.com",
234+
client_metadata=OAuthClientMetadata(
235+
redirect_uris=[AnyUrl("http://localhost:3000/callback")],
236+
),
237+
storage=SimpleTokenStorage(),
238+
idp_token_endpoint="https://your-idp.com/oauth2/v1/token",
239+
token_exchange_params=token_exchange_params,
240+
)
241+
242+
_ = httpx.AsyncClient(auth=enterprise_auth, timeout=30.0)
243+
244+
# Use the client for MCP operations...
245+
# ... time passes and token expires ...
246+
247+
# When token expires, get a fresh ID token from your IdP
248+
new_id_token = await get_id_token_from_idp()
249+
250+
# Refresh the authentication using the helper method
251+
await enterprise_auth.refresh_with_new_id_token(new_id_token)
252+
253+
# Next API call will automatically use the refreshed tokens
254+
# No need to recreate the client or session
255+
256+
214257
if __name__ == "__main__":
215258
asyncio.run(main())

0 commit comments

Comments
 (0)