Skip to content

Commit 0e498d3

Browse files
committed
fix(auth): parse fractional expires_in and stop treating expiry 0 as no expiry
calculate_token_expiry declares int|str but calls int(), so a server emitting a fractional expires_in as a string raises ValueError during token setup, and a float input is truncated instead of kept. Parse with float() so both work. is_token_valid tested token_expiry_time for truthiness, so an expiry of 0.0 was read as 'no expiry' and the token was presented forever. Compare against None explicitly. Tests cover None, int, numeric string, fractional string and float inputs, plus the zero-expiry predicate.
1 parent a525a3d commit 0e498d3

4 files changed

Lines changed: 75 additions & 6 deletions

File tree

‎src/mcp/client/auth/oauth2.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,10 @@ def is_token_valid(self) -> bool:
188188
return bool(
189189
self.current_tokens
190190
and self.current_tokens.access_token
191-
and (not self.token_expiry_time or time.time() <= self.token_expiry_time)
191+
# None means the authorization server declared no expiry. Compare against that
192+
# explicitly: a falsy test would also treat an expiry of 0.0 (the epoch, long
193+
# expired) as "no expiry" and keep presenting a dead token.
194+
and (self.token_expiry_time is None or time.time() <= self.token_expiry_time)
192195
)
193196

194197
def can_refresh_token(self) -> bool:

‎src/mcp/shared/auth_utils.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,18 @@ def check_resource_allowed(requested_resource: str, configured_resource: str) ->
6565
return requested_path.startswith(configured_path)
6666

6767

68-
def calculate_token_expiry(expires_in: int | str | None) -> float | None:
68+
def calculate_token_expiry(expires_in: int | float | str | None) -> float | None:
6969
"""Calculate token expiry timestamp from expires_in seconds.
7070
7171
Args:
72-
expires_in: Seconds until token expiration (may be string from some servers)
72+
expires_in: Seconds until token expiration (may be a string or a float from some servers)
7373
7474
Returns:
7575
Unix timestamp when token expires, or None if no expiry specified
7676
"""
7777
if expires_in is None:
7878
return None # pragma: no cover
79-
# Defensive: handle servers that return expires_in as string
80-
return time.time() + int(expires_in)
79+
# Defensive: handle servers that return expires_in as a string, including ones that
80+
# send a fractional value such as "3599.5". int() would raise ValueError on those and
81+
# silently truncate the float case, expiring the token up to a second early.
82+
return time.time() + float(expires_in)

‎tests/client/test_auth.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,19 @@ async def test_token_validity_checking(self, oauth_provider: OAuthClientProvider
253253
context.client_info = None
254254
assert not context.can_refresh_token()
255255

256+
@pytest.mark.anyio
257+
async def test_token_expiry_of_zero_is_expired(self, oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken):
258+
"""An expiry timestamp of 0 means expired, not 'no expiry'.
259+
260+
`token_expiry_time` is a float|None where None means the server declared no
261+
expiry. Testing it for truthiness conflates 0.0 (the epoch, long expired) with
262+
absence, so a token whose expiry resolves to 0 is treated as never expiring.
263+
"""
264+
context = oauth_provider.context
265+
context.current_tokens = valid_tokens
266+
context.token_expiry_time = 0.0
267+
assert not context.is_token_valid()
268+
256269
def test_clear_tokens(self, oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken):
257270
"""Test clear_tokens() removes token data."""
258271
context = oauth_provider.context

‎tests/shared/test_auth_utils.py‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
"""Tests for OAuth 2.0 Resource Indicators utilities."""
22

3+
import time
4+
35
from pydantic import HttpUrl
46

5-
from mcp.shared.auth_utils import check_resource_allowed, resource_url_from_server_url
7+
from mcp.shared.auth_utils import calculate_token_expiry, check_resource_allowed, resource_url_from_server_url
68

79
# Tests for resource_url_from_server_url function
810

@@ -121,3 +123,52 @@ def test_check_resource_allowed_empty_paths():
121123
assert check_resource_allowed("https://example.com", "https://example.com") is True
122124
assert check_resource_allowed("https://example.com/", "https://example.com") is True
123125
assert check_resource_allowed("https://example.com/api", "https://example.com") is True
126+
127+
128+
# Tests for calculate_token_expiry function
129+
130+
131+
def test_calculate_token_expiry_none_returns_none():
132+
"""A server that declares no expiry yields None, not a timestamp."""
133+
assert calculate_token_expiry(None) is None
134+
135+
136+
def test_calculate_token_expiry_integer_seconds():
137+
"""The common case: expires_in arrives as an integer."""
138+
before = time.time()
139+
expiry = calculate_token_expiry(3600)
140+
after = time.time()
141+
assert expiry is not None
142+
assert before + 3600 <= expiry <= after + 3600
143+
144+
145+
def test_calculate_token_expiry_numeric_string():
146+
"""Some servers emit expires_in as a JSON string."""
147+
before = time.time()
148+
expiry = calculate_token_expiry("3600")
149+
after = time.time()
150+
assert expiry is not None
151+
assert before + 3600 <= expiry <= after + 3600
152+
153+
154+
def test_calculate_token_expiry_fractional_string():
155+
"""A fractional string must round, not raise.
156+
157+
The declared signature accepts str, and the docstring promises the string case is
158+
handled defensively, but int("3599.5") raises ValueError. Servers that send a
159+
fractional expires_in as a string fail token setup instead of expiring 0.5s later.
160+
"""
161+
before = time.time()
162+
expiry = calculate_token_expiry("3599.5")
163+
after = time.time()
164+
assert expiry is not None
165+
assert before + 3599.5 <= expiry <= after + 3599.5
166+
167+
168+
def test_calculate_token_expiry_float():
169+
"""A float expires_in keeps its fraction instead of being truncated."""
170+
before = time.time()
171+
expiry = calculate_token_expiry(3599.5)
172+
after = time.time()
173+
assert expiry is not None
174+
assert before + 3599.5 <= expiry <= after + 3599.5

0 commit comments

Comments
 (0)