Skip to content

Commit 9be3ef7

Browse files
committed
fix(auth): tell a replayed-and-dead token apart from a skewed clock
An elapsed claim has two causes and they want opposite answers. The previous commit treated both as skew. replayed the server keeps handing back one cached token and its `exp` has finally passed. The token really is dead. Falling back to `expires_in` gives a corpse a fresh 300s lease -- the pre-fix behaviour -- and leaves the 401 retry to pay a round trip that a proactive mint would have avoided. first seen the clocks disagree. The server validates `exp` against its own clock, so the token is fine and only this client disbelieves it. Honouring the claim pins `_exp` in the past and runs both grants on every request forever. `_replayed`, set two lines above, already separates them: it is True only for a token this manager already held, i.e. one whose claim it accepted as FUTURE on an earlier mint -- so the clock cannot be the explanation the second time. Under skew each mint returns a distinct token, so the fallback still applies. The union -- skewed clock AND replaying server -- takes the honour-the-claim branch and goes back to both grants per request. Traded knowingly: rarer than either alone, and the 401 retry still covers it. The elapsed-claim path had coverage only in the skew direction; the replay test stopped at a claim 10s in the future, so nothing pinned down what happened when that token finally elapsed. The new test drives `_mint` directly rather than waiting out the first fallback's 300s lease, since that lease is the thing under test and not a precondition worth sleeping for. 234 passed. Live endpoint tracks exactly (300/300 ... 290/290).
1 parent 5d0c0ad commit 9be3ef7

2 files changed

Lines changed: 49 additions & 8 deletions

File tree

‎hotdata/_auth.py‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -367,16 +367,33 @@ def _mint(self, params):
367367
now = time.time()
368368
by_expires_in = now + expires_in
369369
claim = _exp_from_jwt(token)
370-
if claim is not None and claim > now:
370+
if claim is not None and (claim > now or self._replayed):
371371
self._exp = min(by_expires_in, claim)
372372
else:
373-
# No readable claim, or one ALREADY ELAPSED when the mint returned.
374-
# An elapsed claim means the clocks disagree, not that the server
375-
# handed back a corpse -- it validates `exp` against its own clock, so
376-
# the token is fine and only this client disbelieves it. Honouring the
377-
# claim here would set `_exp` permanently in the past, and every
378-
# request would then run both grants forever. `expires_in` is always
379-
# in the future, which is what makes it the safe fallback for skew.
373+
# No readable claim, or one already elapsed on a token we had NOT seen
374+
# before. Two causes produce an elapsed claim and they want opposite
375+
# answers, so `_replayed` -- set just above -- separates them:
376+
#
377+
# replayed the server keeps handing back one cached token, and its
378+
# `exp` has finally passed. The token really is dead.
379+
# Honour the claim: this manager accepted the same claim as
380+
# FUTURE on an earlier mint, so the clock is not the
381+
# explanation, and pretending otherwise would put `_exp` at
382+
# `now + 300` on a corpse -- the pre-fix behaviour, leaving
383+
# the 401 retry to pay a round trip that a proactive mint
384+
# would have avoided.
385+
#
386+
# first seen the clocks disagree. The server validates `exp` against
387+
# its own clock, so the token is fine and only this client
388+
# disbelieves it. Honouring the claim would pin `_exp` in
389+
# the past and run both grants on every request forever.
390+
# `expires_in` is always in the future, which is what makes
391+
# it the safe fallback.
392+
#
393+
# A skewed clock AND a replaying server together fall to the first
394+
# branch, i.e. back to both grants per request. That union is the case
395+
# being traded away, knowingly: it is rarer than either alone and the
396+
# 401 retry still covers it.
380397
self._exp = by_expires_in
381398
self._refresh = data.get("refresh_token") or self._refresh
382399
return True

‎tests/test_auth.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,3 +1213,27 @@ def test_a_claim_already_elapsed_is_declined_so_a_skewed_clock_still_works() ->
12131213
mgr.bearer_value()
12141214
assert len(pool.calls) == calls, "kept minting under skew"
12151215

1216+
1217+
def test_a_replayed_token_that_finally_elapsed_is_not_given_a_fresh_lease() -> None:
1218+
"""The other cause of an elapsed claim, which wants the opposite answer to
1219+
skew. When the server keeps replaying one cached token, its `exp` eventually
1220+
passes for real. Falling back to `expires_in` there would put `_exp` at
1221+
`now + 300` on a corpse -- the pre-fix behaviour -- and leave the 401 retry to
1222+
pay a round trip a proactive mint would have avoided.
1223+
1224+
Drives `_mint` directly: reaching this through `bearer_value` would need the
1225+
first fallback's 300s lease to run down first, which is the thing under test,
1226+
not a precondition worth sleeping for.
1227+
"""
1228+
dead = _jwt_with_exp(-5, jti="replayed-and-dead")
1229+
pool = _FakePool([_mint_response(access_token=dead, expires_in=300)])
1230+
mgr = _TokenManager("hd_secret_token", _config(), pool=pool)
1231+
grant = {"grant_type": "api_token", "api_token": "hd_secret_token"}
1232+
1233+
mgr._mint(dict(grant)) # first sight: elapsed but unseen -> skew branch
1234+
assert mgr._replayed is False
1235+
assert mgr._exp - time.time() > 200, "first sight should fall back to expires_in"
1236+
1237+
mgr._mint(dict(grant)) # same token back -> a known replay
1238+
assert mgr._replayed is True
1239+
assert mgr._exp - time.time() < 0, "gave a dead replayed token a fresh lease"

0 commit comments

Comments
 (0)