Skip to content

OAuth token expiry equality boundary causes intermittent downstream test failures #3729

Open

Description

@BlueLi66

Initial Checks

Release line

2.x (current stable)

Description

While running FastMCP's stored-token expiry tests on Windows, I observed intermittent failures with expires_in=0: the client sometimes reused the stored token instead of acquiring a fresh one.

I reproduced the behavior in isolated environments using MCP SDK 2.3.0 alone and FastMCP 4.1.1 with MCP SDK 2.3.0.

OAuthContext.is_token_valid() returns True when the current time exactly equals token_expiry_time, because it uses <=. Once the clock advances past that timestamp, it correctly returns False. The same equality behavior occurs with a normal 3600-second token checked exactly at expiry.

I expected the token to be invalid at its expiration timestamp. Could you confirm whether the inclusive boundary is intentional, or whether it should be considered expired at equality?

The observed impact is intermittent downstream test failures, not a demonstrated production authentication failure. The example below uses a fixed nonzero clock to make the boundary deterministic; it needs no network or real credentials.

Prepared with AI assistance; the reproductions were executed locally and reviewed before submission.

Example Code

from unittest.mock import Mock, patch

from mcp.client.auth.oauth2 import OAuthContext, TokenStorage
from mcp.shared.auth import OAuthClientMetadata, OAuthToken

context = OAuthContext(
    server_url="https://mcp.example.test/mcp",
    client_metadata=OAuthClientMetadata(
        redirect_uris=["http://localhost/callback"]
    ),
    storage=Mock(spec=TokenStorage),
    redirect_handler=None,
    callback_handler=None,
)

with patch("time.time", return_value=1_800_000_000.0):
    token = OAuthToken(
        access_token="example-token",
        token_type="Bearer",
        expires_in=0,
    )
    context.current_tokens = token
    context.update_token_expiry(token)
    print("At expiry:", context.is_token_valid())  # True

with patch("time.time", return_value=1_800_000_000.001):
    print("After expiry:", context.is_token_valid())  # False

Python & MCP Python SDK

Windows
Python 3.12.15
MCP SDK 2.3.0
Pydantic 2.14.1

Also reproduced through FastMCP 4.1.1's client-credentials auth flow
with MCP SDK 2.3.0 in a separate environment.

Activity

  1. added
    bugSomething isn't working
    v2Affects the v2 line (2.x on main)
    v1Affects the v1.x maintenance line
    on Oct 18, 2026
  2. sattyamjjain commented on Oct 20, 2026

    @sattyamjjain

    I think this is the <= at expiry. At 2af9e30, is_token_valid() in src/mcp/client/auth/oauth2.py (L186-192) checks time.time() <= self.token_expiry_time, and calculate_token_expiry in src/mcp/shared/auth_utils.py (L68-80) returns time.time() + int(expires_in). With expires_in=0, the expiry equals the current time, so if the next time.time() read returns the same value, the token counts as valid: L613 skips the refresh, and L622 adds the auth header.

    RFC 7519 section 4.1.4 defines exp as the time "on or after which the JWT MUST NOT be accepted for processing", so < matches the spec better than <=.

    On why it's intermittent and Windows-only: Python 3.13 changed time.time() on Windows to GetSystemTimePreciseAsFileTime(). You're on 3.12, where the clock is coarser, so two reads returning the same value is much more likely. That is my explanation; I haven't reproduced it.

    May be worth refreshing a few seconds early too, but changing <= to < should fix this case on its own.

  3. pei711 commented on Oct 20, 2026

    @pei711

    Hi maintainers, I've investigated this issue and verified that RFC 7519 §4.1.4 specifies a token is invalid on or after the exp timestamp (i.e. now >= exp). In src/mcp/client/auth/oauth2.py and identity_assertion.py, changing expires_at <= now to < now treats the boundary timestamp as expired rather than valid, preventing intermittent downstream race conditions where the expiry timestamp matches the current clock tick.

    I have a patch with tests ready and verified against the auth test suite and linters. Could you please assign this issue to me so the PR guard allows reopening/submitting? Thanks!

  4. linhongyu510 commented on Oct 22, 2026

    @linhongyu510

    Hi maintainers — following up on @sattyamjjain's <= diagnosis and @pei711's ready patch in #3730 (closed by the require-linked-issue bot because the issue was unassigned, not on technical grounds). I'd like to respect that earlier claim and build on it rather than duplicate it.

    An independent hard-oracle check on the strict < change went 9/12 baseline → 12/12 after fix → 9/12 on revert (the 3 exact-boundary cases raise AssertionError without the fix). The official auth suite passes 210 passed / 1 xfailed. The change touches the two time.time() <= expiry sites — OAuthContext.is_token_valid() (oauth2.py) and the cached-Bearer gate in IdentityAssertionOAuthProvider._auth_flow (identity_assertion.py) — and preserves negative expires_in, missing expiry, the still-unexpired reuse path, and the no-token path; the epoch-seconds time contract is unchanged. This aligns with RFC 7519 §4.1.4 (invalid on/after exp).

    I have not opened a PR or pushed anything, to respect the assignment gate. Could a maintainer give direction, assign #3729, or reopen #3730 once eligible? Happy to collaborate with @pei711 on whichever path maintainers prefer.

    Disclosure: this patch was drafted with AI assistance and verified locally; RFC interpretation and scope remain for maintainers to confirm.

  5. Ben-Home commented on Oct 26, 2026

    @Ben-Home

    The client and the server disagree at the boundary by up to a full second, so the fix is an exclusive comparison plus a refresh leeway rather than a bare flip.

    Measured on current main:

    • src/mcp/client/auth/oauth2.py:191 in is_token_valid() reads time.time() <= self.token_expiry_time, so the client calls a token valid at the exact expiry instant and keeps doing so while the float is greater.
    • src/mcp/shared/auth_utils.py:80 builds that token_expiry_time as time.time() + int(expires_in), a float, so the client compares float against float.
    • The server paths are strict and truncated: src/mcp/server/auth/handlers/token.py:156 uses auth_code.expires_at < time.time(), and src/mcp/server/auth/middleware/bearer_auth.py:71 uses auth_info.expires_at < int(time.time()).

    The int() is the part that matters. The server floors its own clock to whole seconds while the client does not, so for the final sub-second before a token expires the client's predicate returns True and the server's returns False. That window exists on every token, does not need exact float equality to occur, and lands as an intermittent 401 that no layer above can reproduce on demand. RFC 7519's exp reading agrees with the server: the token must not be accepted on or after that time, so equality means expired.

    So two things belong in the same change. Make the client exclusive at equality so it matches both the spec and the server, then add a refresh leeway with a small non-zero default (30 to 60 seconds is the usual range) so refresh fires before the boundary instead of racing it. The leeway is the half that fixes deployments, because the caller who sees this 401 is never the one who configured a margin. Keeping a zero default available preserves strict semantics for anyone asserting on it.

    On tests: freeze the clock and assert both sides explicitly, now == expiry - 1 valid and now == expiry invalid, plus the sub-second case where now is expiry - 0.4 and the server-side integer compare already fails. That third case is the one that reproduces the reported flake; the first two only pin the comparison choice. If the clock is injectable, add the resync case as well, where the host clock jumps across the boundary after the token was accepted, since that is the sequence that shows up as an unexplained 401 in production rather than in a test suite.

    If maintainers would rather not take a behaviour change on the stable line, the leeway alone is still worth landing: it makes the boundary unreachable in practice without touching the documented semantics.

  6. Ben-Home commented on Oct 26, 2026

    @Ben-Home

    Correction to my first comment, after I replaced reasoning with an enumeration.

    I claimed that the client's <= combined with the server's int(time.time()) floor leaves a window where the client considers a token valid and the bearer middleware rejects it. I enumerated that window over four issue instants and three expires_in values at 50 ms steps and it is empty. The floor runs in the safe direction: expires_at < int(now) compares the expiry against a number that is never larger than now, so the middleware is at least as lenient as the client, by up to one second. The client expires first in 1498 of the sampled instants, which costs one unnecessary refresh and never a 401. The boundary is not the cause of the flake, and the direction of my first comment was inverted.

    Two client-side lenient paths remain, and both are worth more than the boundary:

    1. expires_in absent. calculate_token_expiry returns None, and is_token_valid reads that as no expiry at all (not self.token_expiry_time), so a server that issues a token with an expiry but omits expires_in gets a client that never refreshes and a 401 at the server's own clock. This is the only path I can find where the client is unconditionally more permissive than the server, and it matches an intermittent 401 that reproduces against one authorization server and not another.

    2. calculate_token_expiry crashes on a decimal string. The docstring says the string case is handled defensively, but the body is int(expires_in), and int("3599.5") raises ValueError. A server that emits expires_in as a JSON string with a fractional part fails token setup instead of rounding. That one is a real defect with a one line fix, and it belongs in a PR rather than in this thread.

    What would settle the original report fastest: the expires_in value in the failing token response, and whether it is absent, an integer, or a string. Those three cases produce three different failures and only the first one is client-lenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingv1Affects the v1.x maintenance linev2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions