|
| 1 | +# Feature Specification: OAuth Token Refresh Reliability |
| 2 | + |
| 3 | +**Feature Branch**: `023-oauth-state-persistence` |
| 4 | +**Created**: 2026-01-12 |
| 5 | +**Status**: Draft |
| 6 | +**Input**: Fix token refresh so OAuth servers survive restarts and proactively refresh before expiration |
| 7 | + |
| 8 | +## Problem Statement |
| 9 | + |
| 10 | +OAuth-enabled MCP servers require manual re-authentication after any mcpproxy downtime (restart, laptop sleep, weekend). Despite having refresh tokens stored in the database, the automatic token refresh is not working reliably. |
| 11 | + |
| 12 | +**Root causes identified through investigation:** |
| 13 | + |
| 14 | +1. **Misleading logging**: Current "OAuth token refresh successful" logs are incorrect - they log when `client.Start()` returns nil, not when an actual token refresh occurs. The logged duration (1 nanosecond) is impossible for a real HTTP refresh call. |
| 15 | + |
| 16 | +2. **Refresh errors are swallowed**: The mcp-go library's `getValidToken()` function attempts refresh but swallows the error, making it impossible to diagnose why refresh failed (expired refresh token, network error, provider rejection, etc.). |
| 17 | + |
| 18 | +3. **No startup recovery**: When mcpproxy starts with already-expired tokens, there is no mechanism to attempt refresh before connecting. |
| 19 | + |
| 20 | +4. **Access tokens are short-lived**: Tokens expire in approximately 2 hours, requiring reliable proactive refresh to maintain connectivity. |
| 21 | + |
| 22 | +## User Scenarios & Testing *(mandatory)* |
| 23 | + |
| 24 | +### User Story 1 - Survive Laptop Sleep/Wake (Priority: P1) |
| 25 | + |
| 26 | +As a developer using mcpproxy, I want OAuth servers to automatically reconnect after my laptop wakes from sleep, so I don't have to manually re-authenticate every morning. |
| 27 | + |
| 28 | +**Why this priority**: This is the most common disruption - every developer experiences this daily. It's the primary pain point driving this feature. |
| 29 | + |
| 30 | +**Independent Test**: Can be fully tested by authenticating an OAuth server, sleeping the laptop for 3+ hours (allowing tokens to expire), waking it, and verifying the server automatically reconnects without user intervention. |
| 31 | + |
| 32 | +**Acceptance Scenarios**: |
| 33 | + |
| 34 | +1. **Given** an authenticated OAuth server with a valid refresh token, **When** the laptop sleeps for 3 hours and wakes, **Then** the server automatically refreshes the token and reconnects within 30 seconds. |
| 35 | +2. **Given** an authenticated OAuth server, **When** mcpproxy is restarted, **Then** the server automatically reconnects using the stored refresh token without requiring browser authentication. |
| 36 | + |
| 37 | +--- |
| 38 | + |
| 39 | +### User Story 2 - Proactive Token Refresh (Priority: P2) |
| 40 | + |
| 41 | +As a developer using mcpproxy during a long work session, I want tokens to be refreshed automatically before they expire, so my MCP tools never fail mid-task due to token expiration. |
| 42 | + |
| 43 | +**Why this priority**: Prevents disruption during active work. Without this, tokens could expire during tool calls causing unexpected failures. |
| 44 | + |
| 45 | +**Independent Test**: Can be tested by authenticating an OAuth server, monitoring token expiration time, and verifying that a new token is obtained before the original expires. |
| 46 | + |
| 47 | +**Acceptance Scenarios**: |
| 48 | + |
| 49 | +1. **Given** an authenticated OAuth server with a token expiring in 2 hours, **When** 80% of the token lifetime elapses (approximately 96 minutes), **Then** the system automatically refreshes the token without user action. |
| 50 | +2. **Given** a proactive refresh is scheduled, **When** the refresh succeeds, **Then** a new refresh is scheduled for 80% of the new token's lifetime. |
| 51 | +3. **Given** a proactive refresh is scheduled, **When** the refresh fails, **Then** the user is notified with the specific error reason. |
| 52 | + |
| 53 | +--- |
| 54 | + |
| 55 | +### User Story 3 - Clear Refresh Failure Feedback (Priority: P3) |
| 56 | + |
| 57 | +As a developer, when automatic token refresh fails, I want to understand why it failed, so I can take appropriate action (re-authenticate vs. wait for network vs. contact provider). |
| 58 | + |
| 59 | +**Why this priority**: Improves debuggability and user experience. Currently, all refresh failures show the same generic "Authentication required" message. |
| 60 | + |
| 61 | +**Independent Test**: Can be tested by simulating various refresh failure scenarios and verifying distinct, actionable error messages appear in health status and logs. |
| 62 | + |
| 63 | +**Acceptance Scenarios**: |
| 64 | + |
| 65 | +1. **Given** a refresh token that has been revoked by the provider, **When** refresh is attempted, **Then** the health status shows "Refresh token expired - re-authentication required" with the `login` action. |
| 66 | +2. **Given** a network connectivity issue, **When** refresh is attempted, **Then** the health status shows "Refresh failed - network error" with the `retry` action. |
| 67 | +3. **Given** any refresh failure, **When** viewing logs, **Then** the actual OAuth error (e.g., `invalid_grant`, `server_error`) is logged with full context. |
| 68 | + |
| 69 | +--- |
| 70 | + |
| 71 | +### Edge Cases |
| 72 | + |
| 73 | +- What happens when the refresh token itself has expired on the provider side? |
| 74 | + - System should detect this (e.g., `invalid_grant` error) and prompt for re-authentication rather than retrying indefinitely. |
| 75 | + |
| 76 | +- How does the system handle rapid successive refresh attempts? |
| 77 | + - System should implement rate limiting to prevent overwhelming the OAuth provider (minimum 10 seconds between refresh attempts per server). |
| 78 | + |
| 79 | +- What happens if refresh succeeds but the new token is immediately invalid? |
| 80 | + - System should detect this on the subsequent connection attempt and report the specific error rather than entering a retry loop. |
| 81 | + |
| 82 | +- What happens during a network partition? |
| 83 | + - System should use exponential backoff for refresh retries, with clear status indicating retry attempts. |
| 84 | + |
| 85 | +## Requirements *(mandatory)* |
| 86 | + |
| 87 | +### Functional Requirements |
| 88 | + |
| 89 | +- **FR-001**: System MUST attempt to refresh expired tokens at startup before attempting to connect OAuth servers. |
| 90 | +- **FR-002**: System MUST use the mcp-go library's `RefreshToken()` method directly to obtain actual refresh errors (not swallowed errors). |
| 91 | +- **FR-003**: System MUST log accurate token refresh results, including actual duration and success/failure status with error details. |
| 92 | +- **FR-004**: System MUST schedule proactive token refresh at 80% of token lifetime to prevent expiration during use. |
| 93 | +- **FR-005**: System MUST persist newly refreshed tokens to the database immediately after successful refresh. |
| 94 | +- **FR-006**: System MUST display distinct health status messages for different refresh failure types (expired refresh token, network error, provider error). |
| 95 | +- **FR-007**: System MUST remove or correct the current misleading "OAuth token refresh successful" logging that reports success when `Start()` returns nil. |
| 96 | +- **FR-008**: System MUST rate-limit refresh attempts to no more than one per 10 seconds per server. |
| 97 | + |
| 98 | +### Key Entities |
| 99 | + |
| 100 | +- **OAuth Token**: Access token, refresh token, expiration timestamp, token type, scope. Stored in database with server identifier. |
| 101 | +- **Refresh Schedule**: Server name, scheduled refresh time, retry count, last error. Managed by RefreshManager. |
| 102 | +- **Health Status**: Level (healthy/degraded/unhealthy), summary, detail (including refresh error), suggested action. |
| 103 | + |
| 104 | +## Success Criteria *(mandatory)* |
| 105 | + |
| 106 | +### Measurable Outcomes |
| 107 | + |
| 108 | +- **SC-001**: OAuth servers automatically reconnect within 60 seconds after mcpproxy restart, without requiring user re-authentication (when refresh tokens are valid). |
| 109 | +- **SC-002**: Tokens are refreshed before expiration 99% of the time during continuous operation, preventing mid-session authentication failures. |
| 110 | +- **SC-003**: When refresh fails, users see the specific failure reason (not generic "Authentication required") within 5 seconds of the failure. |
| 111 | +- **SC-004**: Log messages accurately reflect refresh attempt outcomes - no false "successful" logs for failed or unattempted refreshes. |
| 112 | +- **SC-005**: After implementation, manual re-authentication is required only when refresh tokens have genuinely expired on the provider side (not due to system bugs). |
| 113 | + |
| 114 | +## Assumptions |
| 115 | + |
| 116 | +- The mcp-go library v0.44.0-beta.2 (or later stable version) will be used, which provides the `GetOAuthHandler()` method and improved OAuth error detection. |
| 117 | +- OAuth providers honor standard RFC 6749 refresh token semantics. |
| 118 | +- Refresh tokens remain valid for at least 24 hours on most providers (provider-specific expiration is outside our control). |
| 119 | +- Network connectivity is generally available; brief outages should be handled with retry logic. |
| 120 | + |
| 121 | +## Future Work |
| 122 | + |
| 123 | +- **offline_access scope**: Investigate adding `offline_access` to scope requests for providers that support it (Atlassian, Google) to potentially obtain longer-lived refresh tokens. |
| 124 | +- **Token expiration warnings**: Add health status indicators when tokens are approaching expiration without scheduled refresh. |
| 125 | +- **Refresh token rotation handling**: Some providers issue new refresh tokens on each use; ensure proper handling of token rotation. |
| 126 | + |
| 127 | +## Commit Message Conventions *(mandatory)* |
| 128 | + |
| 129 | +When committing changes for this feature, follow these guidelines: |
| 130 | + |
| 131 | +### Issue References |
| 132 | +- **Use**: `Related #[issue-number]` - Links the commit to the issue without auto-closing |
| 133 | +- **Do NOT use**: `Fixes #[issue-number]`, `Closes #[issue-number]`, `Resolves #[issue-number]` - These auto-close issues on merge |
| 134 | + |
| 135 | +**Rationale**: Issues should only be closed manually after verification and testing in production, not automatically on merge. |
| 136 | + |
| 137 | +### Co-Authorship |
| 138 | +- **Do NOT include**: `Co-Authored-By: Claude <noreply@anthropic.com>` |
| 139 | +- **Do NOT include**: "Generated with Claude Code" |
| 140 | + |
| 141 | +**Rationale**: Commit authorship should reflect the human contributors, not the AI tools used. |
| 142 | + |
| 143 | +### Example Commit Message |
| 144 | +``` |
| 145 | +feat(oauth): implement startup token refresh |
| 146 | +
|
| 147 | +Related #XXX |
| 148 | +
|
| 149 | +Adds automatic token refresh at startup for OAuth servers with expired |
| 150 | +access tokens but valid refresh tokens. |
| 151 | +
|
| 152 | +## Changes |
| 153 | +- Add startup refresh logic in RefreshManager |
| 154 | +- Call mcp-go RefreshToken() directly for error visibility |
| 155 | +- Update health status to show refresh failure reasons |
| 156 | +
|
| 157 | +## Testing |
| 158 | +- Verified with expired Google OAuth tokens |
| 159 | +- Confirmed proper error surfacing for invalid_grant |
| 160 | +``` |
0 commit comments