modelcontextprotocol / modelcontextprotocol/python-sdk

OAuth client can never recover from an expired DCR client secret — even though the SDK's own server issues and enforces one

Đang mở
#3,256 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

bug P1 v1 v2
Ngôn ngữ chính
Python
Star
24.3k
Fork
4k
Merge trung bình
1 ngày 1 giờ
Pull request đã merge (30 ngày)
31

Mô tả

Initial Checks
Release line

2.x (current stable) — the code paths below were verified on main @ a4f4ccd09113 and are identical on v1.29.0; the production observation happened on 1.28.1 (provenance).

Description
Summary

The SDK ships both halves of a trap, and they don't fit together.

As a server, the SDK can issue expiring DCR client secrets and rejects them once lapsed:

step code (main @ a4f4ccd09113)
knob ClientRegistrationOptions.client_secret_expiry_seconds: int | None = Nonesrc/mcp/server/auth/settings.py:6
issuance client_secret_expires_at = client_id_issued_at + options.client_secret_expiry_secondssrc/mcp/server/auth/handlers/register.py:123-129
enforcement if client.client_secret_expires_at and ... < int(time.time()): raise AuthenticationError("Client secret has expired")src/mcp/server/auth/middleware/client_auth.py:116-117
wire response AuthenticationError{"error": "invalid_client", "error_description": "Client secret has expired"}src/mcp/server/auth/handlers/token.py:106-113

As a client, OAuthClientProvider persists client_secret_expires_at through its own TokenStorage abstraction and then never looks at it — and treats invalid_client as a plain fatal error:

  • Scanning every .py file under src/mcp/client/ (23 files at a4f4ccd09113) finds zero occurrences of client_secret_expires_at and zero of invalid_client. Same for v1.29.0's oauth2.py.
  • Registration only happens when stored client info is absent (if not self.context.client_info: in async_auth_flow, "Step 4"). Expired-but-present client info is reused forever.
  • _handle_token_response raises OAuthTokenError on any non-200; nothing invalidates the dead registration.

Consequence: point an OAuthClientProvider at a server that enforces secret expiry — including a server built with this very SDK and client_secret_expiry_seconds set — and once the window passes, the client is permanently stuck:

  1. access token expires → refresh presents the expired client_secretinvalid_client → SDK falls back to full authorization
  2. full authorization succeeds (the user consents in the browser — the authorization leg is unaffected) → token exchange presents the same expired secret → invalid_client again
  3. the next attempt reloads the same client info from storage → registration skipped → back to 1

User-visible symptom: "I re-authenticated and nothing changed." No amount of consent helps. Recovery requires the application to reach into the TokenStorage payload and delete the persisted client info by hand — nothing in the API surface hints that a storage payload can become permanently toxic.

Expiring secrets and the absence of a rotation endpoint are both squarely within spec (RFC 7591 makes client_secret_expires_at REQUIRED whenever a secret is issued, with 0 meaning "never expires"; RFC 7592 is optional and this SDK's server doesn't implement it either — its routes are /authorize, /register, /revoke, /token and metadata only). In that world, re-registration is the only standard recovery path, and the SDK client is the only party that can perform it — the server can't push a new secret, user consent doesn't touch client authentication, and the application above the SDK doesn't manage client_info at all.

Real-world occurrence

freee (a major Japanese accounting SaaS) runs an official remote MCP server at https://mcp.freee.co.jp/mcp whose DCR issues secrets that expire 30 days after issuance (observable from its public /register endpoint). Our production client (mcp 1.28.1) worked for exactly 30 days and then hard-failed exactly as above — with the same error string this SDK's server produces:

POST /token  (grant_type=refresh_token, expired client credentials)
→ {"error":"invalid_client","error_description":"Client secret has expired"}

Client authentication is checked before the grant, so every token-endpoint interaction dies once the secret lapses (the same registration with live credentials and a deliberately bogus grant returns invalid_grant instead — confirming the ordering).

Expected behavior

Two small, complementary changes on the client side:

  1. Treat stored client info with an expired secret as absent. Before using persisted client info, check client_secret_expires_at (non-zero and in the past → discard, fall through to registration / CIMD). main already validates freshly returned registrations for usability via check_registration_usable; extending that notion to stored info seems natural.
  2. Invalidate stored client info when the server answers invalid_client. Per RFC 6749 §5.2 that means client authentication failed — for a dynamically registered client, the registration is dead by definition. Clearing it (and the tokens bound to it) lets the next flow re-register instead of failing identically forever. Note the HTTP status varies in practice (this SDK's server returns 401, the server we hit returns 400), so keying on the error field rather than the status code is the robust form.

(1) fixes the stuck state proactively whenever the AS declares the expiry; (2) also covers servers that expire or revoke registrations without declaring it.

We currently implement both as an application-side wrapper around TokenStorage / the provider, which turned recovery from "impossible" into "one re-consent" in production.

Verification notes
  • Code paths above were read at main a4f4ccd09113 (2.x) and v1.29.0 (1.x); the "zero readers" claims come from scanning all 23 client files, not a spot check
  • The stuck loop and the invalid_client / invalid_grant ordering were observed against the third-party server named above
  • Not verified by us: an end-to-end SDK-server ↔ SDK-client reproduction (we did not stand up an SDK server with client_secret_expiry_seconds set). The claim there rests on the code paths in the table — setting the knob to a few seconds and letting an OAuthClientProvider outlive it should confirm it quickly
Example Code
# Deterministic client-side reproduction (no 30-day wait): hand the provider a
# storage that is already in the post-expiry state, then run any flow against a
# server that enforces expiry (e.g. an SDK server with
# ClientRegistrationOptions(client_secret_expiry_seconds=<small>)).

import time

from mcp.client.auth import TokenStorage
from mcp.shared.auth import OAuthClientInformationFull, OAuthToken


class ExpiredClientStorage(TokenStorage):
    """Storage state after the AS-issued client secret lapsed."""

    def __init__(self) -> None:
        self._client_info = OAuthClientInformationFull(
            client_id="registered-client-id",
            client_secret="expired-secret",
            client_secret_expires_at=int(time.time()) - 3600,  # already expired
            redirect_uris=["http://localhost:3030/callback"],
            token_endpoint_auth_method="client_secret_post",
        )
        self._tokens: OAuthToken | None = None

    async def get_tokens(self) -> OAuthToken | None:
        return self._tokens

    async def set_tokens(self, tokens: OAuthToken) -> None:
        self._tokens = tokens

    async def get_client_info(self) -> OAuthClientInformationFull | None:
        return self._client_info

    async def set_client_info(self, info: OAuthClientInformationFull) -> None:
        self._client_info = info


# Attach to OAuthClientProvider and connect:
#   - registration is skipped (Step 4 sees client info present),
#   - authorization succeeds, token exchange fails with invalid_client,
#   - every subsequent attempt repeats identically — there is no path back.
Versions
  • Observed in production: mcp 1.28.1, Python 3.14, streamable HTTP transport
  • Code-inspected as still present: v1.29.0 (latest 1.x) and main @ a4f4ccd09113 (2.x)

✍️ Author: Claude Code with @carrotRakko (AI-written, human-approved)

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với OAuthClientProvider.async_auth_flow và _handle_token_response trong src/mcp/client/, sau đó kiểm tra TokenStorage và check_registration_usable. Theo dõi cách client_info đã lưu và các phản hồi invalid_client được xử lý. Hoàn tất khi dữ liệu đăng ký đã hết hạn hoặc bị từ chối được xóa, một flow sau đó có thể đăng ký lại và client không còn lặp lại vòng lặp xác thực thất bại.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python
Lĩnh vực
api, authentication
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
68/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.