diff --git a/README.ja.md b/README.ja.md index 3153353..6544eca 100644 --- a/README.ja.md +++ b/README.ja.md @@ -172,6 +172,9 @@ mcp-stdio [OPTIONS] URL --oauth-device OAuth 2.1 Device Authorization Grant（RFC 8628）— ヘッドレス環境向け --client-id ID 事前登録済み OAuth クライアント ID（MCP_OAUTH_CLIENT_ID 環境変数でも指定可） --oauth-scope SCOPE 要求する OAuth スコープ + --oauth-refresh-leeway SECONDS + アクセストークンを expire の何秒前に proactive refresh + するか（デフォルト: 60、または MCP_OAUTH_REFRESH_LEEWAY 環境変数） -H, --header 'Key: Value' カスタムヘッダー（複数指定可） --transport {streamable-http,sse} トランスポート種別（デフォルト: streamable-http） diff --git a/README.md b/README.md index 5849315..a3d262a 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,9 @@ Options: --oauth-device Enable OAuth 2.1 Device Authorization Grant (RFC 8628, headless) --client-id ID Pre-registered OAuth client ID (or set MCP_OAUTH_CLIENT_ID) --oauth-scope SCOPE OAuth scope to request + --oauth-refresh-leeway SECONDS + Proactively refresh tokens this many seconds before + expiry (default: 60, or MCP_OAUTH_REFRESH_LEEWAY) -H, --header 'Key: Value' Custom header (can be repeated) --transport {streamable-http,sse} Transport type (default: streamable-http) diff --git a/WORKAROUNDS.md b/WORKAROUNDS.md index efa9c74..6329cd5 100644 --- a/WORKAROUNDS.md +++ b/WORKAROUNDS.md @@ -34,6 +34,7 @@ Works around known issues in Claude Code's HTTP transport: - **OAuth discovery does not fall back when 401 carries a non-`Bearer` `WWW-Authenticate`** — the MCP TypeScript SDK's `StreamableHTTPClientTransport` only falls back to well-known PRM discovery when the 401 response has no `WWW-Authenticate` header at all; a `Negotiate` or other non-Bearer challenge is bubbled up as a hard failure, so servers that expose both SPNEGO and OAuth (common in Windows-integrated enterprise deployments) cannot initiate the OAuth flow even though valid metadata is hosted at the spec-defined well-known URI ([typescript-sdk#1946](https://github.com/modelcontextprotocol/typescript-sdk/issues/1946)). mcp-stdio performs discovery pre-flight rather than on 401: it probes `/.well-known/oauth-protected-resource` (path-aware, then host-root) and `/.well-known/oauth-authorization-server` directly, so the fallback is not gated on the `WWW-Authenticate` scheme and the OAuth flow proceeds regardless of what authentication schemes the server additionally advertises. - **Cannot connect to servers that only support static bearer tokens** — mcp-remote always initiates an OAuth discovery handshake before any tool call; servers that authenticate via static bearer tokens (e.g. [Zabbix MCP Server](https://github.com/initMAX/zabbix-mcp-server)) respond with 404 on `/.well-known/oauth-authorization-server` and the proxy gives up before the bearer-auth path is ever reached ([zabbix-mcp-server#36](https://github.com/initMAX/zabbix-mcp-server/issues/36)); mcp-stdio connects directly with `--bearer-token YOUR_TOKEN http://your-server:8080/mcp`, skipping OAuth entirely. - **OAuth token exchange fails with URL-encoded responses** — TypeScript SDK's token exchange assumes the response is always JSON; servers that return `application/x-www-form-urlencoded` (e.g. GitHub OAuth) cause a JSON parse error, blocking authentication ([typescript-sdk#759](https://github.com/modelcontextprotocol/typescript-sdk/issues/759)); mcp-stdio's `_parse_token_response()` checks the `Content-Type` header and parses `application/x-www-form-urlencoded` via `urllib.parse.parse_qs`, so GitHub MCP and similar servers work without extra configuration. +- **No proactive token refresh window** — mcp-remote (and `adaptOAuthProvider` in TypeScript SDK) only refreshes the access token after a 401 has already fired, with no early-refresh leeway. ASes that issue refresh tokens whose lifetime is barely longer than the access token's leave no margin for clock skew, so a refresh attempt can race the token expiry and fail ([mcp-remote#252](https://github.com/geelen/mcp-remote/issues/252), [typescript-sdk#1954](https://github.com/modelcontextprotocol/typescript-sdk/issues/1954)). mcp-stdio's `ensure_token()` performs proactive refresh: a cached access token is treated as expired when its expiry is within `--oauth-refresh-leeway` seconds (default 60, configurable via flag or `MCP_OAUTH_REFRESH_LEEWAY`), so the refresh hits well before the AS revokes the token. - **No OAuth support in headless/SSH environments** — mcp-remote's OAuth flow requires opening a browser window, making it unusable in SSH sessions, CI/CD pipelines, or other browserless environments; there is no Device Authorization Grant support ([mcp-remote#228](https://github.com/geelen/mcp-remote/issues/228)). mcp-stdio supports RFC 8628 Device Authorization Grant via `--oauth-device`: it displays a short user code and verification URI on stderr so the user can authenticate from any browser, while the device polls the token endpoint in the background. - **`resource` indicator gets a trailing slash appended** — TypeScript SDK normalises the resource URL via `new URL(...).href`, converting `https://api.example.com` to `https://api.example.com/`; Atlassian authv2 and similar servers reject this with `InvalidTargetError: Incorrect resource parameters` ([typescript-sdk#1968](https://github.com/modelcontextprotocol/typescript-sdk/issues/1968), [mcp-remote#261](https://github.com/geelen/mcp-remote/issues/261)); mcp-stdio passes `resource=server_url` verbatim in both code exchange and refresh requests — Python's URL handling does not add trailing slashes. diff --git a/src/mcp_stdio/cli.py b/src/mcp_stdio/cli.py index d921408..31bc7db 100644 --- a/src/mcp_stdio/cli.py +++ b/src/mcp_stdio/cli.py @@ -13,6 +13,17 @@ from . import __version__ from .relay import check_connection, log, run, run_sse +def _non_negative_float(value: str) -> float: + """argparse type for non-negative floats (rejects negative leeway).""" + try: + f = float(value) + except ValueError as exc: + raise argparse.ArgumentTypeError(f"invalid float value: {value!r}") from exc + if f < 0: + raise argparse.ArgumentTypeError(f"value must be >= 0 (got {f})") + return f + + # RFC 7230 §3.2.6 field-name = token = 1*tchar. tchar covers # "!#$%&'*+-.^_`|~" plus DIGIT and ALPHA. Used to reject header names # that could be misinterpreted by downstream HTTP parsers. @@ -166,6 +177,22 @@ def main() -> None: default="", help="OAuth scope to request", ) + parser.add_argument( + "--oauth-refresh-leeway", + type=_non_negative_float, + # Pass as string so argparse re-applies _non_negative_float to the + # default — invalid env var values (negative, non-numeric) surface + # as argparse errors instead of a raw Python ValueError on startup. + default=os.environ.get("MCP_OAUTH_REFRESH_LEEWAY", "60"), + metavar="SECONDS", + help=( + "Proactively refresh access tokens this many seconds before they " + "expire (default: 60, or MCP_OAUTH_REFRESH_LEEWAY env var). " + "Increase for ASes with significant clock skew; decrease for " + "deployments where short-lived tokens make a 60 s window " + "exceed token TTL" + ), + ) parser.add_argument( "-H", "--header", @@ -290,6 +317,7 @@ def main() -> None: client_id=args.client_id or None, scope=args.oauth_scope or None, device_flow=args.oauth_device, + refresh_leeway=args.oauth_refresh_leeway, ) headers["Authorization"] = f"Bearer {token_data.access_token}" token_refresher = _build_token_refresher( diff --git a/src/mcp_stdio/oauth.py b/src/mcp_stdio/oauth.py index 33535e6..610e784 100644 --- a/src/mcp_stdio/oauth.py +++ b/src/mcp_stdio/oauth.py @@ -978,20 +978,27 @@ def ensure_token( scope: str | None = None, timeout: float = 120, device_flow: bool = False, + refresh_leeway: float = 60.0, ) -> TokenData: """Ensure a valid access token is available. - 1. Check cached token — use if not expired + 1. Check cached token — use if not expired (with ``refresh_leeway`` margin) 2. If expired, try refresh 3. If no token or refresh fails, run OAuth flow: - ``device_flow=True``: Device Authorization Grant (RFC 8628) - ``device_flow=False``: Authorization Code flow with PKCE (default) + ``refresh_leeway`` is the proactive-refresh window in seconds: a cached + token is considered expired when its actual expiry is within this many + seconds from now. Default 60 s absorbs typical clock skew. Tune via + ``--oauth-refresh-leeway`` for ASes that issue extremely short-lived + access tokens or for deployments with larger clock skew. + Returns TokenData with a valid access_token. """ cached = load_token(server_url) if cached and cached.access_token: - if cached.expires_at is None or cached.expires_at > time.time() + 60: + if cached.expires_at is None or cached.expires_at > time.time() + refresh_leeway: log("using cached OAuth token") return cached diff --git a/tests/test_cli.py b/tests/test_cli.py index 75010c4..a891e33 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -135,6 +135,75 @@ def test_custom_timeouts(self): assert kwargs.kwargs["timeout_connect"] == 5.0 assert kwargs.kwargs["timeout_read"] == 60.0 + def test_oauth_refresh_leeway_default(self, monkeypatch): + """#56: --oauth-refresh-leeway defaults to 60 s when env var unset and flag absent.""" + monkeypatch.delenv("MCP_OAUTH_REFRESH_LEEWAY", raising=False) + with ( + patch("sys.argv", ["mcp-stdio", "--oauth", "https://example.com/mcp"]), + patch("mcp_stdio.oauth.ensure_token") as mock_ensure, + patch("mcp_stdio.cli.run"), + ): + mock_ensure.return_value.access_token = "tok" + main() + assert mock_ensure.call_args.kwargs["refresh_leeway"] == 60.0 + + def test_oauth_refresh_leeway_custom_flag(self): + """#56: --oauth-refresh-leeway flag is propagated to ensure_token.""" + with ( + patch( + "sys.argv", + [ + "mcp-stdio", + "--oauth", + "--oauth-refresh-leeway", + "300", + "https://example.com/mcp", + ], + ), + patch("mcp_stdio.oauth.ensure_token") as mock_ensure, + patch("mcp_stdio.cli.run"), + ): + mock_ensure.return_value.access_token = "tok" + main() + assert mock_ensure.call_args.kwargs["refresh_leeway"] == 300.0 + + def test_oauth_refresh_leeway_env_var(self, monkeypatch): + """#56: MCP_OAUTH_REFRESH_LEEWAY env var is respected when flag absent.""" + monkeypatch.setenv("MCP_OAUTH_REFRESH_LEEWAY", "120") + with ( + patch("sys.argv", ["mcp-stdio", "--oauth", "https://example.com/mcp"]), + patch("mcp_stdio.oauth.ensure_token") as mock_ensure, + patch("mcp_stdio.cli.run"), + ): + mock_ensure.return_value.access_token = "tok" + main() + assert mock_ensure.call_args.kwargs["refresh_leeway"] == 120.0 + + def test_oauth_refresh_leeway_negative_rejected(self, capsys): + """#56: negative leeway values are rejected at parse time.""" + with patch( + "sys.argv", + [ + "mcp-stdio", + "--oauth-refresh-leeway", + "-1", + "https://example.com/mcp", + ], + ): + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code == 2 # argparse error + assert "must be >= 0" in capsys.readouterr().err + + def test_oauth_refresh_leeway_invalid_env_var_rejected(self, monkeypatch, capsys): + """#56: invalid env var values surface as argparse errors, not ValueError.""" + monkeypatch.setenv("MCP_OAUTH_REFRESH_LEEWAY", "not-a-number") + with patch("sys.argv", ["mcp-stdio", "https://example.com/mcp"]): + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code == 2 + assert "invalid float value" in capsys.readouterr().err + def test_oauth_and_bearer_token_mutually_exclusive(self): with patch( "sys.argv", diff --git a/tests/test_oauth.py b/tests/test_oauth.py index 06a6945..39cb3f6 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -1363,6 +1363,96 @@ def test_uses_cached_valid_token(self, tmp_path, monkeypatch): data = ensure_token("https://example.com/mcp", client) assert data.access_token == "cached_at" + def test_refresh_leeway_zero_uses_actual_expiry(self, tmp_path, monkeypatch): + """#56: refresh_leeway=0 disables proactive refresh — token valid until literal expiry. + + Default leeway (60 s) would treat a token expiring in 30 s as expired + and trigger refresh. With leeway=0, the cached token is used until the + actual expires_at moment. + """ + store_file = tmp_path / "tokens.json" + monkeypatch.setattr("mcp_stdio.token_store._STORE_DIR", tmp_path) + monkeypatch.setattr("mcp_stdio.token_store._STORE_FILE", store_file) + + from mcp_stdio.token_store import save_token + + save_token( + "https://example.com/mcp", + TokenData( + access_token="short_lived_at", + expires_at=time.time() + 30, # within default 60 s leeway + refresh_token="rt", + client_id="cid", + token_endpoint="https://example.com/token", + authorization_endpoint="https://example.com/authorize", + ), + ) + + client = httpx.Client() + data = ensure_token("https://example.com/mcp", client, refresh_leeway=0) + assert data.access_token == "short_lived_at" # used as-is, no refresh + + def test_refresh_leeway_large_triggers_proactive_refresh( + self, tmp_path, monkeypatch, httpx_mock + ): + """#56: large refresh_leeway proactively refreshes even when token has time left.""" + store_file = tmp_path / "tokens.json" + monkeypatch.setattr("mcp_stdio.token_store._STORE_DIR", tmp_path) + monkeypatch.setattr("mcp_stdio.token_store._STORE_FILE", store_file) + + from mcp_stdio.token_store import save_token + + save_token( + "https://example.com/mcp", + TokenData( + access_token="cached_at", + expires_at=time.time() + 200, # 200 s left — exceeds default leeway + refresh_token="valid_rt", + client_id="cid", + token_endpoint="https://example.com/token", + authorization_endpoint="https://example.com/authorize", + ), + ) + + # leeway=300 — 200 < 300, treated as near-expiry → refresh + httpx_mock.add_response( + url="https://example.com/token", + json={"access_token": "refreshed_at", "expires_in": 3600}, + ) + + client = httpx.Client() + data = ensure_token("https://example.com/mcp", client, refresh_leeway=300) + assert data.access_token == "refreshed_at" + + def test_refresh_leeway_default_60s(self, tmp_path, monkeypatch, httpx_mock): + """#56: default leeway of 60 s — token expiring in 30 s is refreshed.""" + store_file = tmp_path / "tokens.json" + monkeypatch.setattr("mcp_stdio.token_store._STORE_DIR", tmp_path) + monkeypatch.setattr("mcp_stdio.token_store._STORE_FILE", store_file) + + from mcp_stdio.token_store import save_token + + save_token( + "https://example.com/mcp", + TokenData( + access_token="near_expiry_at", + expires_at=time.time() + 30, # within default leeway + refresh_token="valid_rt", + client_id="cid", + token_endpoint="https://example.com/token", + authorization_endpoint="https://example.com/authorize", + ), + ) + + httpx_mock.add_response( + url="https://example.com/token", + json={"access_token": "refreshed_at", "expires_in": 3600}, + ) + + client = httpx.Client() + data = ensure_token("https://example.com/mcp", client) # default leeway + assert data.access_token == "refreshed_at" + def test_refreshes_expired_token(self, tmp_path, monkeypatch, httpx_mock): """Token expired but refresh_token available.""" store_file = tmp_path / "tokens.json"