Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ Two things are versioned separately from this file and worth knowing about:

## [Unreleased]

### Fixed

- A burst of refreshes on one cookie no longer signs the person out. The reuse
grace window rotated the session again on every grace refresh, so the third
request of a burst matched nothing, got a 401, and its response cleared the
cookie the other two had just set. Within `REFRESH_REUSE_GRACE_SECONDS` a spent
token is now answered with the successor the session already holds - the same
token for every request in the burst - so the cookie converges whichever
response lands last.
- A session that has ended sends the person to sign in. A refused refresh left the
console signed in, with every request answering 401 and the chat socket
reconnecting on a dead token, until a full reload.

## [0.0.516] - 2026-10-01

### Changed
Expand Down
52 changes: 35 additions & 17 deletions backend/app/api/routes/v1/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from app.schemas.token import MagicLinkToken, RefreshTokenRequest, Token
from app.schemas.user import MeRead, UserCreate, UserRead
from app.services.email.service import get_email_service
from app.services.session import refresh_expiry, successor_refresh_token

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -103,9 +104,11 @@ async def refresh_token(
"""Exchange a refresh token for a new access token."""
await enforce_auth_limit(request, surface="auth_refresh")

session = await session_service.validate_refresh_token(
body.refresh_token
) or await session_service.claim_refresh_grace(body.refresh_token)
session = await session_service.validate_refresh_token(body.refresh_token)
within_grace = False
if not session:
session = await session_service.claim_refresh_grace(body.refresh_token)
within_grace = session is not None
if not session:
# Before the refusal, and only on the path where one is already certain:
# a token that validated no live session may be a typo, an expired one, a
Expand Down Expand Up @@ -133,20 +136,35 @@ async def refresh_token(
if payload is None or payload.get("cv", 0) != user.credential_version:
raise AuthenticationError(message="Invalid or expired refresh token")

new_refresh_token = create_refresh_token(
subject=str(user.id), credential_version=user.credential_version
)

# Rotate the refresh token in place, keeping the row's id: the new access
# token names the same `sid`, so a live socket or a second tab holding the
# old access token is not cut off by a routine refresh (#1437, #1501). The
# old refresh token's hash is replaced, which is what makes it unusable.
await session_service.rotate_session(
session,
new_refresh_token,
ip_address=request.client.host if request.client else None,
user_agent=request.headers.get("User-Agent"),
)
if within_grace:
# A token this row spent seconds ago - a lost response, or one request of
# a burst on the same cookie. Answered with the successor the row already
# holds, not a new rotation, so every request in the burst gets the same
# token and the cookie jar converges on it.
new_refresh_token = session_service.reissue_within_grace(
session, body.refresh_token, credential_version=user.credential_version
)
else:
# Rotate the refresh token in place, keeping the row's id: the new access
# token names the same `sid`, so a live socket or a second tab holding the
# old access token is not cut off by a routine refresh (#1437, #1501). The
# old refresh token's hash is replaced, which is what makes it unusable.
# The successor is derived from the spent token, so a grace-window reissue
# can rebuild it byte for byte.
expires_at = refresh_expiry()
new_refresh_token = successor_refresh_token(
body.refresh_token,
subject=str(user.id),
credential_version=user.credential_version,
expires_at=expires_at,
)
await session_service.rotate_session(
session,
new_refresh_token,
expires_at=expires_at,
ip_address=request.client.host if request.client else None,
user_agent=request.headers.get("User-Agent"),
)
access_token = create_access_token(subject=str(user.id), sid=str(session.id))
return Token(access_token=access_token, refresh_token=new_refresh_token)

Expand Down
14 changes: 12 additions & 2 deletions backend/app/core/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ def create_refresh_token(
expires_delta: timedelta | None = None,
*,
credential_version: int = 0,
jti: str | None = None,
expires_at: datetime | None = None,
) -> str:
"""Create a JWT refresh token.

Expand All @@ -69,11 +71,19 @@ def create_refresh_token(
next refresh's `scalar_one_or_none` lookup raises rather than resolving (#1501
review).

A rotation passes `jti` and `expires_at` instead, derived from the token it
spends, so the same spent token always mints the same successor - which is
what lets `SessionService.reissue_within_grace` hand every request in a burst
the one token the row now holds. The payload has no `iat`, so equal inputs
encode to equal bytes.

Carries the account's `credential_version` as `cv`: a password change bumps
the user's version, and the refresh path refuses a token whose `cv` is behind
it, so a token minted before the change cannot be rotated past it (#1517).
"""
if expires_delta:
if expires_at is not None:
expire = expires_at
elif expires_delta:
expire = datetime.now(UTC) + expires_delta
else:
expire = datetime.now(UTC) + timedelta(minutes=settings.REFRESH_TOKEN_EXPIRE_MINUTES)
Expand All @@ -82,7 +92,7 @@ def create_refresh_token(
"exp": expire,
"sub": str(subject),
"type": "refresh",
"jti": uuid4().hex,
"jti": jti or uuid4().hex,
"cv": credential_version,
}
return jwt.encode(to_encode, settings.SECRET_KEY, algorithm=settings.ALGORITHM)
Expand Down
71 changes: 65 additions & 6 deletions backend/app/services/session.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
"""Session service (PostgreSQL async)."""

import hashlib
import hmac
import logging
import secrets
from datetime import UTC, datetime, timedelta
from typing import Any
from uuid import UUID
Expand All @@ -11,7 +13,7 @@
from app.core.audit import record_audit
from app.core.config import settings
from app.core.exceptions import AuthenticationError, NotFoundError
from app.core.security import read_uuid_claim
from app.core.security import create_refresh_token, read_uuid_claim
from app.db.models.session import Session
from app.repositories import session_repo
from app.schemas.session import SessionListResponse, SessionRead
Expand All @@ -24,6 +26,29 @@ def hash_token(token: str) -> str:
return hashlib.sha256(token.encode()).hexdigest()


def successor_refresh_token(
spent_token: str, *, subject: str, credential_version: int, expires_at: datetime
) -> str:
"""The refresh token a rotation of `spent_token` issues - the same bytes every time.

The `jti` is an HMAC of the spent token's hash and `exp` is the row's own
expiry, so the successor can be rebuilt later from what the row stores, without
the row ever holding a credential. Unique along a chain because every spent
token is.
"""
jti = hmac.new(
settings.SECRET_KEY.encode(), hash_token(spent_token).encode(), hashlib.sha256
).hexdigest()[:32]
return create_refresh_token(
subject=subject, credential_version=credential_version, jti=jti, expires_at=expires_at
)


def refresh_expiry() -> datetime:
"""When a refresh token minted now stops refreshing."""
return datetime.now(UTC) + timedelta(minutes=settings.REFRESH_TOKEN_EXPIRE_MINUTES)


def _parse_user_agent(user_agent: str | None) -> tuple[str | None, str | None]:
if not user_agent:
return None, None
Expand Down Expand Up @@ -93,6 +118,7 @@ async def rotate_session(
session: Session,
new_refresh_token: str,
*,
expires_at: datetime | None = None,
ip_address: str | None = None,
user_agent: str | None = None,
) -> Session:
Expand All @@ -108,8 +134,13 @@ async def rotate_session(
recreating the row used to: the sessions list is what a person revokes an
unfamiliar device from, so it has to show where the credential is being used
now, not only where the login began (#1501 review).

`expires_at` must be the `exp` the new token was minted with when it came
from `successor_refresh_token`, or a reissue within the grace window would
rebuild a different token than the row holds.
"""
expires_at = datetime.now(UTC) + timedelta(minutes=settings.REFRESH_TOKEN_EXPIRE_MINUTES)
if expires_at is None:
expires_at = refresh_expiry()
device_name, device_type = _parse_user_agent(user_agent)
return await session_repo.rotate(
self.db,
Expand Down Expand Up @@ -207,8 +238,8 @@ async def claim_refresh_grace(self, refresh_token: str) -> Session | None:
closed mid-request - or when a second tab refreshed on the same cookie a
moment after the first. Treating that as a replay ended the session and
signed the person out several times a day. So inside the window the spent
token refreshes once more, and the route rotates the row again; the next
presentation of it finds a different previous hash and is refused.
token is answered again - by `reissue_within_grace`, with the successor the
row already holds, never with a new rotation.

The window is the cost: a stolen refresh token replayed within seconds of
the victim's own refresh is accepted rather than detected. Outside it,
Expand All @@ -221,8 +252,8 @@ async def claim_refresh_grace(self, refresh_token: str) -> Session | None:
grace = settings.REFRESH_REUSE_GRACE_SECONDS
if grace <= 0:
return None
# Locked like the ordinary lookup, so two grace refreshes on the same
# spent token serialize: the second finds the previous hash moved on.
# Locked like the ordinary lookup, so a grace answer never interleaves
# with a rotation of the same row.
session = await session_repo.get_by_previous_refresh_token_hash(
self.db, hash_token(refresh_token), for_update=True
)
Expand All @@ -241,6 +272,34 @@ async def claim_refresh_grace(self, refresh_token: str) -> Session | None:
logger.info("refresh_token_grace_reuse", extra={"session_id": str(session.id)})
return session

def reissue_within_grace(
self, session: Session, spent_token: str, *, credential_version: int
) -> str:
"""The successor a grace-window refresh answers with: the token the row holds now.

Rotating again here, as the first version of the window did, moved the
previous hash on - so in a burst of three refreshes on one cookie the third
matched nothing, got a 401, and its response cleared the cookie the other
two had just set. The session stayed active and the browser lost it.
Answering every request in the burst with the *same* token lets the cookie
jar converge whichever response lands last, and a client whose response
was lost gets back exactly the token it missed.

Raises:
AuthenticationError: The row has rotated past that successor since - a
later refresh on the new token - or the account's credential
version has moved, so the rebuilt token is not the one it holds.
"""
successor = successor_refresh_token(
spent_token,
subject=str(session.user_id),
credential_version=credential_version,
expires_at=session.expires_at,
)
if not secrets.compare_digest(hash_token(successor), session.refresh_token_hash):
raise AuthenticationError(message="Invalid or expired refresh token")
return successor

async def detect_refresh_reuse(
self, refresh_token: str, *, ip_address: str | None = None
) -> Session | None:
Expand Down
91 changes: 63 additions & 28 deletions backend/tests/integration/test_session_revocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,37 +318,36 @@ async def test_the_refresh_route_ends_the_chain_and_still_answers_401(

class TestTheReuseGraceWindow:
"""A spent refresh token presented seconds after its rotation is a lost
response or a second tab, not a thief. Ending the session for it signed people
out several times a day, so inside `REFRESH_REUSE_GRACE_SECONDS` it refreshes
once more through the real route."""
response or one request of a burst on the same cookie, not a thief. Ending the
session for it signed people out several times a day, so inside
`REFRESH_REUSE_GRACE_SECONDS` it is answered again, through the real route,
with the successor the row already holds."""

async def _rotated(self, db, email: str) -> tuple[UUID, UUID, str, str]:
async def _rotated(self, db, api: AsyncClient, email: str) -> tuple[UUID, str, str]:
"""A session whose first token has been spent by a real refresh."""
user = await _user(db, email)
spent = create_refresh_token(subject=str(user.id), credential_version=0)
current = create_refresh_token(subject=str(user.id), credential_version=0)
session = await session_repo.create(
db, user_id=user.id, refresh_token_hash=hash_token(spent), expires_at=_in_a_day()
)
await SessionService(db).rotate_session(session, current)
return user.id, session.id, spent, current

async def test_a_lost_response_does_not_sign_the_person_out(self, db, api: AsyncClient):
user_id, session_id, spent, _ = await self._rotated(db, "grace-http@example.com")
session_id = session.id
first = await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})
assert first.status_code == 200
db.expire_all()
return session_id, spent, first.json()["refresh_token"]

response = await api.post(
f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent}
)
async def test_a_lost_response_gets_back_the_token_it_missed(self, db, api: AsyncClient):
session_id, spent, successor = await self._rotated(db, api, "grace-lost@example.com")

assert response.status_code == 200
retry = await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})

assert retry.status_code == 200
assert retry.json()["refresh_token"] == successor
db.expire_all()
reread = await session_repo.get_by_id(db, session_id)
assert reread is not None
assert reread.is_active is True
# The token it answered with is the live one now.
service = SessionService(db)
assert await service.validate_refresh_token(response.json()["refresh_token"]) is not None
assert await session_repo.count_user_sessions(db, user_id, open_only=True) == 1
assert reread.refresh_token_hash == hash_token(successor)
recorded = (
await db.execute(
select(func.count())
Expand All @@ -358,26 +357,62 @@ async def test_a_lost_response_does_not_sign_the_person_out(self, db, api: Async
).scalar_one()
assert recorded == 0

async def test_the_spent_token_refreshes_only_once(self, db, api: AsyncClient):
"""The grace rotation moves the previous hash on, so the same spent token
a second time matches nothing: a plain 401, and nothing to revoke."""
_, session_id, spent, _ = await self._rotated(db, "grace-once@example.com")
async def test_every_request_in_a_burst_gets_the_same_token(self, db, api: AsyncClient):
"""The first version of the window rotated again on each grace refresh, so
the third request of a burst matched nothing, got a 401, and its response
cleared the cookie the others had just set. The session stayed active and
the browser lost it."""
_, spent, successor = await self._rotated(db, api, "grace-burst@example.com")

answers = [
await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})
for _ in range(3)
]

assert [answer.status_code for answer in answers] == [200, 200, 200]
assert {answer.json()["refresh_token"] for answer in answers} == {successor}
db.expire_all()
assert await SessionService(db).validate_refresh_token(successor) is not None

first = await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})
again = await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})
async def test_once_the_successor_has_rotated_the_spent_token_is_refused(
self, db, api: AsyncClient
):
"""A later refresh on the new token moves the chain on; the old one no
longer names the token the row holds. A plain 401, and nothing revoked."""
session_id, spent, successor = await self._rotated(db, api, "grace-moved-on@example.com")
onward = await api.post(
f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": successor}
)
assert onward.status_code == 200
db.expire_all()

assert first.status_code == 200
assert again.status_code == 401
late = await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})

assert late.status_code == 401
db.expire_all()
reread = await session_repo.get_by_id(db, session_id)
assert reread is not None
assert reread.is_active is True

async def test_rotation_records_when_it_happened(self, db):
_, session_id, _, _ = await self._rotated(db, "grace-stamp@example.com")
async def test_a_password_change_closes_the_window(self, db, api: AsyncClient):
"""The spent token carries the old credential version, so the route
refuses it before any successor is rebuilt (#1517)."""
session_id, spent, _ = await self._rotated(db, api, "grace-password@example.com")
reread = await session_repo.get_by_id(db, session_id)
assert reread is not None
user = await user_repo.get_by_id(db, reread.user_id)
assert user is not None
user.credential_version = 1
await db.flush()
db.expire_all()

late = await api.post(f"{settings.API_V1_STR}/auth/refresh", json={"refresh_token": spent})

assert late.status_code == 401

async def test_rotation_records_when_it_happened(self, db, api: AsyncClient):
session_id, _, _ = await self._rotated(db, api, "grace-stamp@example.com")

reread = await session_repo.get_by_id(db, session_id)
assert reread is not None
assert reread.rotated_at is not None
Expand Down
Loading
Loading