Skip to content

fix(mcp-oauth): fix token storage serialization and cached token refresh - #5399

Closed
jvg123 wants to merge 1 commit into
NousResearch:mainfrom
jvg123:fix/mcp-oauth-token-refresh
Closed

fix(mcp-oauth): fix token storage serialization and cached token refresh#5399
jvg123 wants to merge 1 commit into
NousResearch:mainfrom
jvg123:fix/mcp-oauth-token-refresh

Conversation

@jvg123

@jvg123 jvg123 commented Apr 6, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes two bugs in tools/mcp_oauth.py that cause MCP OAuth to break in non-interactive environments (gateways, Docker containers, cron jobs). Together these bugs mean any deployment without a browser loses MCP connectivity after ~1 hour and cannot recover, even with a valid refresh token.

Related Issue

Relates to #5344 (redirect URI port reuse addressed separately in #5345).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📚 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎉 New skill (bundled or hub)

Changes Made

  • HermesTokenStorage.set_tokens() and set_client_info(): Changed model_dump(exclude_none=True) to model_dump(mode="json", exclude_none=True). Pydantic AnyUrl fields in the MCP SDK's token/client models aren't serializable by the default JSON encoder, causing TypeError: Object of type AnyUrl is not JSON serializable on first OAuth connection.

  • HermesTokenStorage.get_tokens(): When loading cached tokens from disk, checks whether the access token has expired by comparing file_mtime + expires_in against the current time. If expired, clears access_token (sets to empty string) but preserves refresh_token. This causes the MCP SDK's is_token_valid() to return False and can_refresh_token() to return True, so it correctly uses the refresh flow instead of sending an expired token, receiving 401, and falling through to the full browser authorization flow.

How to Test

  1. Configure an MCP server with auth: oauth in ~/.hermes/config.yaml
  2. Complete the initial browser OAuth flow
  3. Wait for the access token to expire (~1 hour) or manually set the token file's mtime to the past: touch -t 202401010000 ~/.hermes/mcp-tokens/<server>.json
  4. Reconnect to the MCP server — it should auto-refresh using the refresh token without opening a browser
  5. Run the test suite: pytest tests/tools/test_mcp_oauth.py -v

New test cases added:

  • TestAnyUrlSerialization::test_set_tokens_with_anyurl — verifies mode="json" is passed
  • TestAnyUrlSerialization::test_set_client_info_with_anyurl — verifies mode="json" is passed
  • TestExpiredTokenRefresh::test_expired_token_cleared — verifies expired access token is cleared, refresh token preserved
  • TestExpiredTokenRefresh::test_fresh_token_not_cleared — verifies fresh tokens are returned as-is

🤖 Generated with Claude Code assistance

@jvg123
jvg123 force-pushed the fix/mcp-oauth-token-refresh branch 2 times, most recently from 3f1ffa2 to 19b58ba Compare April 6, 2026 03:27
Two fixes for MCP OAuth in tools/mcp_oauth.py:

1. AnyUrl serialization: model_dump(mode='json') in set_tokens() and
   set_client_info() to handle Pydantic AnyUrl objects that aren't
   JSON-serializable by default. Without this, token storage fails on
   first OAuth connection with TypeError.

2. Cached token refresh (critical): get_tokens() now checks if the
   access token has expired by comparing file mtime + expires_in against
   current time. If expired, clears access_token but keeps refresh_token,
   so the MCP SDK uses the refresh flow instead of sending an expired
   token, getting 401, and falling through to full browser auth.

   Without this fix, any non-interactive deployment (gateway, Docker,
   cron) loses MCP connectivity after ~1 hour and cannot recover, even
   with a valid refresh token.

Note: redirect URI port reuse is addressed separately in NousResearch#5345.

Tests added for both fixes.
@jvg123
jvg123 force-pushed the fix/mcp-oauth-token-refresh branch from 19b58ba to 8aa72ec Compare April 6, 2026 03:34
@jvg123
jvg123 marked this pull request as ready for review April 6, 2026 03:36
@jvg123 jvg123 changed the title fix(mcp-oauth): fix token storage serialization, port reuse, and cached token refresh fix(mcp-oauth): fix token storage serialization and cached token refresh Apr 6, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @jvg123. Closing — both fixes in this PR have been addressed on main:

  1. model_dump(mode="json", ...) for Pydantic AnyUrl — landed as part of fix(mcp-oauth): Pydantic serialization + port hardening + cross-origin security + session auto-reconnect #15125 (commit 5fa2f42, salvaged from @amanuel2's fix: serialize Pydantic AnyUrl fields when persisting MCP OAuth state #5677 which was submitted around the same time).

  2. Expired-token refresh trigger — superseded by fix(mcp-oauth): bidirectional auth_flow bridge + absolute expires_at (salvage #12025) #12717 (commit a3a4932). Main now persists absolute expires_at and clamps expires_in to 0 on reload for expired tokens, which achieves the same "force SDK to refresh" effect your access_token="" approach targeted, with correct wall-clock accounting across process restarts.

Both contributors spotted the same serialization issue — credit preserved in the salvage PR.

@teknium1 teknium1 closed this Apr 24, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth area/auth Authentication, OAuth, credential pools labels Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants