fix(auth): give the rotated access token a fifteen-second sunset - #929
Merged
Conversation
Rotation revoked the access token paired with the consumed refresh token the instant the rotation committed. A native client with requests already on the wire got 401 revoked on those, read that as "re-authenticate", rotated again with the refresh token it had just spent, and tripped reuse detection: the device's whole token family died because the client was busy at the boundary. Pull the paired token's expiry in to fifteen seconds instead. It is long enough for an in-flight request on a slow mobile link to finish, including a retry, and short enough that the token gains nothing meaningful over the validity it already had. bearer.ts answers a passed expiry with reason expired, which a client treats as "refresh", not as "sign in again". LEAST(expires_at, $1) takes the minimum inside the single UPDATE, so the write can only ever shorten: a token already inside its last seconds keeps its own expiry, and there is no read-then-write window. The bound value is an ISO-8601 UTC string cast in SQL so the comparison does not shift with the host's timezone. Reuse detection is untouched. A replayed refresh token, or one presented under a spoofed device id, still revokes the family and its access tokens on the spot, and so does the logout path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A native client with several requests in flight at the 24 h boundary sends some of them with the old access token a few hundred milliseconds before the refresh completes. The rotation revoked that token the instant it committed, so those requests came back 401
revoked; the client read that as "sign in again", rotated a second time with the refresh token it had just consumed, tripped reuse detection, and the whole family was revoked. Signed out for having been busy. The audit trail shows it asauth.token.refreshfollowed immediately byauth.bearer.failure reason=revoked.The paired access token now gets a short sunset instead of an instant revoke: its expiry becomes the earlier of its own expiry and now plus fifteen seconds, in one parameter-bound update with
LEAST, so there is no read-then-write window and a token that would have expired sooner keeps its sooner expiry. The bearer check already rejects an expired token, so nothing changes on the read side beyond the reason.Untouched on purpose: reuse detection. A replayed or device-mismatched refresh token still revokes the family and its access tokens instantly; that path is the stolen-token defence and must not gain a window. Logout still revokes instantly.
Reproduced first against the current code, which failed at the first assertion with
revoked. Four cases: accepted inside the window andexpiredafter it, replay still instant, a five-second expiry stays five seconds, logout instant. Restoring the instant revoke turns three tests red; dropping the minimum turns the five-second case red against real Postgres. The refresh endpoint's contract text never described the paired token's fate, so it is unchanged.