feat: enhance refresh token handling with grace period and rotation detection
Chore App Build, Test, and Push Docker Images / build-and-push (push) Successful in 3m17s

This commit is contained in:
2026-07-26 18:35:56 -04:00
parent 11f68c7f76
commit d4800be3b7
7 changed files with 413 additions and 33 deletions
+1
View File
@@ -10,3 +10,4 @@ backend/test-results/
**/.DS_Store
frontend/cert.pem
frontend/key.pem
tmp/
+1 -1
View File
@@ -5,7 +5,7 @@ description: "Executes a bugfix pipeline on one or more gitea issues: Developer
## What I do
I orchestrate a sequential bugfix and verification pipeline - I will retrieve issues(s) from Gitea. I will then forward information from the issues to the respective subagents.
I orchestrate a sequential bugfix and verification pipeline - I will retrieve issues(s) from Gitea (title, body, images, comments, etc...). I will then forward information from the issues to the respective subagents.
Use gitea-mcp-server to interact with Gitea. Verify that the server is running and accessible.
+67 -22
View File
@@ -43,6 +43,12 @@ try:
ACCESS_TOKEN_EXPIRY_MINUTES = int(os.environ.get('ACCESS_TOKEN_EXPIRY_MINUTES', '15'))
except ValueError:
ACCESS_TOKEN_EXPIRY_MINUTES = 15
try:
REFRESH_TOKEN_REUSE_GRACE_PERIOD_SECONDS = int(
os.environ.get('REFRESH_TOKEN_REUSE_GRACE_PERIOD_SECONDS', '30')
)
except ValueError:
REFRESH_TOKEN_REUSE_GRACE_PERIOD_SECONDS = 30
E2E_TEST_EMAIL = 'e2e@test.com'
E2E_TEST_PASSWORD = 'E2eTestPass1!'
E2E_TEST_PIN = '1234'
@@ -412,18 +418,21 @@ def refresh():
token_record = RefreshToken.from_dict(token_dict)
# THEFT DETECTION: token was already used (rotated out) but replayed
if token_record.is_used:
logger.warning(
'Refresh token reuse detected! user_id=%s, family=%s, ip=%s — killing all sessions',
token_record.user_id, token_record.token_family, request.remote_addr,
)
# Nuke ALL refresh tokens for this user
refresh_tokens_db.remove(TokenQuery.user_id == token_record.user_id)
resp = jsonify({'error': 'Token reuse detected, all sessions invalidated', 'code': REFRESH_TOKEN_REUSE})
# Look up the user early (needed for both legitimate rotation and grace-period handling)
user_dict = users_db.get(UserQuery.id == token_record.user_id)
user = User.from_dict(user_dict) if user_dict else None
if not user:
refresh_tokens_db.remove(TokenQuery.id == token_record.id)
resp = jsonify({'error': 'User not found', 'code': USER_NOT_FOUND})
_clear_auth_cookies(resp)
return resp, 401
if user.marked_for_deletion:
refresh_tokens_db.remove(TokenQuery.user_id == user.id)
resp = jsonify({'error': 'Account marked for deletion', 'code': ACCOUNT_MARKED_FOR_DELETION})
_clear_auth_cookies(resp)
return resp, 403
# Check expiry
try:
exp = datetime.fromisoformat(token_record.expires_at)
@@ -440,23 +449,59 @@ def refresh():
_clear_auth_cookies(resp)
return resp, 401
# Look up the user
user_dict = users_db.get(UserQuery.id == token_record.user_id)
user = User.from_dict(user_dict) if user_dict else None
if not user:
refresh_tokens_db.remove(TokenQuery.id == token_record.id)
resp = jsonify({'error': 'User not found', 'code': USER_NOT_FOUND})
# THEFT DETECTION: token was already used (rotated out) but replayed
if token_record.is_used:
# Grace period: tolerate a very recent rotation to avoid false positives
# from legitimate concurrent refresh requests (race conditions).
grace_period = current_app.config.get(
'REFRESH_TOKEN_REUSE_GRACE_PERIOD_SECONDS', REFRESH_TOKEN_REUSE_GRACE_PERIOD_SECONDS
)
rotated_at = token_record.rotated_at
is_race_condition = False
if rotated_at:
try:
rotated_dt = datetime.fromisoformat(rotated_at)
if rotated_dt.tzinfo is None:
rotated_dt = rotated_dt.replace(tzinfo=timezone.utc)
if (datetime.now(timezone.utc) - rotated_dt).total_seconds() <= grace_period:
is_race_condition = True
except ValueError:
pass
if is_race_condition:
logger.info(
'Refresh token replay within grace period treated as race condition. user_id=%s, family=%s, ip=%s',
token_record.user_id, token_record.token_family, request.remote_addr,
)
raw_new_refresh, _ = _create_refresh_token(user.id, token_family=token_record.token_family)
access_token = _create_access_token(user)
resp = jsonify({
'email': user.email,
'id': user.id,
'first_name': user.first_name,
'last_name': user.last_name,
'verified': user.verified,
})
_set_auth_cookies(resp, access_token, raw_new_refresh)
return resp, 200
logger.warning(
'Refresh token reuse detected! user_id=%s, family=%s, ip=%s — killing family sessions',
token_record.user_id, token_record.token_family, request.remote_addr,
)
# Invalidate only the affected family, not every session for the user.
refresh_tokens_db.remove(
(TokenQuery.user_id == token_record.user_id) & (TokenQuery.token_family == token_record.token_family)
)
resp = jsonify({'error': 'Token reuse detected, family sessions invalidated', 'code': REFRESH_TOKEN_REUSE})
_clear_auth_cookies(resp)
return resp, 401
if user.marked_for_deletion:
refresh_tokens_db.remove(TokenQuery.user_id == user.id)
resp = jsonify({'error': 'Account marked for deletion', 'code': ACCOUNT_MARKED_FOR_DELETION})
_clear_auth_cookies(resp)
return resp, 403
# ROTATION: mark old token as used, create new one in same family
refresh_tokens_db.update({'is_used': True}, TokenQuery.id == token_record.id)
refresh_tokens_db.update(
{'is_used': True, 'rotated_at': datetime.now(timezone.utc).isoformat()},
TokenQuery.id == token_record.id,
)
raw_new_refresh, _ = _create_refresh_token(user.id, token_family=token_record.token_family)
# Issue new access token
+3
View File
@@ -9,6 +9,7 @@ class RefreshToken(BaseModel):
token_family: str = ''
expires_at: str = ''
is_used: bool = False
rotated_at: str | None = None
def to_dict(self):
return {
@@ -18,6 +19,7 @@ class RefreshToken(BaseModel):
'token_family': self.token_family,
'expires_at': self.expires_at,
'is_used': self.is_used,
'rotated_at': self.rotated_at,
}
@staticmethod
@@ -31,4 +33,5 @@ class RefreshToken(BaseModel):
token_family=data.get('token_family', ''),
expires_at=data.get('expires_at', ''),
is_used=data.get('is_used', False),
rotated_at=data.get('rotated_at'),
)
+244 -3
View File
@@ -1,11 +1,11 @@
import pytest
from werkzeug.security import generate_password_hash, check_password_hash
from flask import Flask
from api.auth_api import auth_api
from api.auth_api import auth_api, _hash_token
from db.db import users_db, refresh_tokens_db
from tinydb import Query
from models.user import User
from datetime import datetime
from datetime import datetime, timedelta, timezone
from tests.conftest import TEST_SECRET_KEY, TEST_REFRESH_TOKEN_EXPIRY_DAYS
@pytest.fixture
@@ -188,4 +188,245 @@ def test_migration_script_hashes_plain_text_passwords():
# Check user2 password unchanged
user2_dict = users_db.get(Query().email == 'test2@example.com')
assert user2_dict['password'] == already_hashed
assert user2_dict['password'] == already_hashed
def _extract_cookie_value(response, cookie_name):
"""Extract a raw cookie value from a response's Set-Cookie headers."""
for cookie in response.headers.getlist('Set-Cookie'):
if cookie.startswith(f'{cookie_name}='):
return cookie.split(f'{cookie_name}=', 1)[1].split(';', 1)[0]
return None
def _create_verified_user(email, password):
"""Insert a verified user with the given credentials."""
users_db.remove(Query().email == email)
user = User(
first_name='Test',
last_name='User',
email=email,
password=generate_password_hash(password),
verified=True,
)
users_db.insert(user.to_dict())
return user
def _set_refresh_cookie(client, raw_token):
"""
Set the refresh token cookie on a test client so it is sent to /auth/refresh.
Production uses path='/api/auth' because the frontend calls /api/auth/refresh,
but the test fixture exposes the blueprint at /auth/refresh directly.
"""
client.set_cookie('refresh_token', raw_token, path='/')
def test_refresh_rotates_token(client):
"""A successful refresh marks the old token used and issues a new one in the same family."""
email = 'refresh-rotate@test.com'
password = 'password123'
_create_verified_user(email, password)
login_response = client.post('/auth/login', json={'email': email, 'password': password})
assert login_response.status_code == 200
old_refresh = _extract_cookie_value(login_response, 'refresh_token')
assert old_refresh
_set_refresh_cookie(client, old_refresh)
refresh_response = client.post('/auth/refresh')
assert refresh_response.status_code == 200
new_refresh = _extract_cookie_value(refresh_response, 'refresh_token')
assert new_refresh
assert new_refresh != old_refresh
user_dict = users_db.get(Query().email == email)
old_hash = _hash_token(old_refresh)
new_hash = _hash_token(new_refresh)
old_record = refresh_tokens_db.get(Query().token_hash == old_hash)
assert old_record is not None
assert old_record['is_used'] is True
assert old_record['rotated_at'] is not None
new_record = refresh_tokens_db.get(Query().token_hash == new_hash)
assert new_record is not None
assert new_record['is_used'] is False
assert new_record['token_family'] == old_record['token_family']
def test_refresh_reuse_only_invalidates_family(client):
"""Replay of a used refresh token only kills its own family, not other devices."""
email = 'refresh-family@test.com'
password = 'password123'
user = _create_verified_user(email, password)
# Device A logs in
client_a = client
login_a = client_a.post('/auth/login', json={'email': email, 'password': password})
assert login_a.status_code == 200
refresh_a = _extract_cookie_value(login_a, 'refresh_token')
# Device B logs in (separate client = separate cookie jar)
app = client_a.application
client_b = app.test_client()
login_b = client_b.post('/auth/login', json={'email': email, 'password': password})
assert login_b.status_code == 200
refresh_b = _extract_cookie_value(login_b, 'refresh_token')
assert refresh_a != refresh_b
# Device A refreshes normally
_set_refresh_cookie(client_a, refresh_a)
refresh_a_response = client_a.post('/auth/refresh')
assert refresh_a_response.status_code == 200
# Capture families before any purge so we can assert afterwards.
family_a = refresh_tokens_db.get(Query().token_hash == _hash_token(refresh_a))['token_family']
family_b = refresh_tokens_db.get(Query().token_hash == _hash_token(refresh_b))['token_family']
assert family_a != family_b
# Backdate rotation so the replay is past the grace period and treated as theft.
old_hash_a = _hash_token(refresh_a)
backdated = (datetime.now(timezone.utc) - timedelta(seconds=60)).isoformat()
refresh_tokens_db.update({'rotated_at': backdated}, Query().token_hash == old_hash_a)
# Attacker replays device A's old token
_set_refresh_cookie(client_a, refresh_a)
reuse_response = client_a.post('/auth/refresh')
assert reuse_response.status_code == 401
assert reuse_response.json['code'] == 'REFRESH_TOKEN_REUSE'
# Device B's refresh token should still be valid
_set_refresh_cookie(client_b, refresh_b)
refresh_b_response = client_b.post('/auth/refresh')
assert refresh_b_response.status_code == 200
# Only family A should be purged; family B should remain
remaining = refresh_tokens_db.search(Query().user_id == user.id)
remaining_families = {t['token_family'] for t in remaining}
assert family_a not in remaining_families
assert family_b in remaining_families
def test_refresh_reuse_within_grace_period_is_tolerated(client):
"""A replay within the grace period is treated as a race condition, not theft."""
email = 'refresh-race@test.com'
password = 'password123'
_create_verified_user(email, password)
login_response = client.post('/auth/login', json={'email': email, 'password': password})
assert login_response.status_code == 200
refresh_token = _extract_cookie_value(login_response, 'refresh_token')
# First refresh marks the token as used
_set_refresh_cookie(client, refresh_token)
first_refresh = client.post('/auth/refresh')
assert first_refresh.status_code == 200
# Immediate replay (same legitimate client racing) should succeed
_set_refresh_cookie(client, refresh_token)
race_response = client.post('/auth/refresh')
assert race_response.status_code == 200
# The family should still be valid
user_dict = users_db.get(Query().email == email)
family = refresh_tokens_db.get(Query().token_hash == _hash_token(refresh_token))['token_family']
family_tokens = refresh_tokens_db.search(
(Query().user_id == user_dict['id']) & (Query().token_family == family)
)
assert len(family_tokens) >= 1
assert any(t['is_used'] is False for t in family_tokens)
def test_refresh_reuse_after_grace_period_invalidates_family(client):
"""A replay after the grace period is treated as theft and kills only that family."""
email = 'refresh-theft@test.com'
password = 'password123'
user = _create_verified_user(email, password)
login_response = client.post('/auth/login', json={'email': email, 'password': password})
assert login_response.status_code == 200
refresh_token = _extract_cookie_value(login_response, 'refresh_token')
# Refresh once, then backdate the rotation timestamp past the grace period
_set_refresh_cookie(client, refresh_token)
client.post('/auth/refresh')
old_hash = _hash_token(refresh_token)
old_record = refresh_tokens_db.get(Query().token_hash == old_hash)
old_family = old_record['token_family']
backdated = (datetime.now(timezone.utc) - timedelta(seconds=60)).isoformat()
refresh_tokens_db.update({'rotated_at': backdated}, Query().token_hash == old_hash)
# Replay now should be detected as theft
_set_refresh_cookie(client, refresh_token)
reuse_response = client.post('/auth/refresh')
assert reuse_response.status_code == 401
assert reuse_response.json['code'] == 'REFRESH_TOKEN_REUSE'
remaining = refresh_tokens_db.search(Query().user_id == user.id)
remaining_families = {t['token_family'] for t in remaining}
assert old_family not in remaining_families
def test_refresh_reuse_without_rotated_at_invalidates_family(client):
"""Legacy used tokens without rotated_at are treated as theft, not race conditions."""
email = 'refresh-legacy@test.com'
password = 'password123'
user = _create_verified_user(email, password)
login_response = client.post('/auth/login', json={'email': email, 'password': password})
assert login_response.status_code == 200
refresh_token = _extract_cookie_value(login_response, 'refresh_token')
# Rotate the token, then strip rotated_at to simulate pre-migration data
_set_refresh_cookie(client, refresh_token)
client.post('/auth/refresh')
old_hash = _hash_token(refresh_token)
refresh_tokens_db.update({'rotated_at': None}, Query().token_hash == old_hash)
old_record = refresh_tokens_db.get(Query().token_hash == old_hash)
old_family = old_record['token_family']
# Replay should be treated as theft because rotated_at is missing
_set_refresh_cookie(client, refresh_token)
reuse_response = client.post('/auth/refresh')
assert reuse_response.status_code == 401
assert reuse_response.json['code'] == 'REFRESH_TOKEN_REUSE'
remaining = refresh_tokens_db.search(Query().user_id == user.id)
remaining_families = {t['token_family'] for t in remaining}
assert old_family not in remaining_families
def test_refresh_reuse_with_zero_grace_period(client):
"""A grace period of 0 means any replay of a used token is treated as theft."""
email = 'refresh-zero-grace@test.com'
password = 'password123'
user = _create_verified_user(email, password)
# Configure the app with a 0-second grace period
client.application.config['REFRESH_TOKEN_REUSE_GRACE_PERIOD_SECONDS'] = 0
login_response = client.post('/auth/login', json={'email': email, 'password': password})
assert login_response.status_code == 200
refresh_token = _extract_cookie_value(login_response, 'refresh_token')
# Rotate the token; rotated_at is within the normal default grace period
_set_refresh_cookie(client, refresh_token)
client.post('/auth/refresh')
old_hash = _hash_token(refresh_token)
old_record = refresh_tokens_db.get(Query().token_hash == old_hash)
old_family = old_record['token_family']
# Immediate replay should still be theft with a 0-second grace period
_set_refresh_cookie(client, refresh_token)
reuse_response = client.post('/auth/refresh')
assert reuse_response.status_code == 401
assert reuse_response.json['code'] == 'REFRESH_TOKEN_REUSE'
remaining = refresh_tokens_db.search(Query().user_id == user.id)
remaining_families = {t['token_family'] for t in remaining}
assert old_family not in remaining_families
@@ -6,6 +6,25 @@ vi.mock('@/stores/auth', () => ({
logoutUser: () => mockLogoutUser(),
}))
function makeLocalStorageStub() {
const store: Record<string, string> = {}
return {
getItem: (key: string) => store[key] ?? null,
setItem: (key: string, value: string) => {
store[key] = value
},
removeItem: (key: string) => {
delete store[key]
},
clear: () => {
for (const k of Object.keys(store)) delete store[k]
},
_store: store,
}
}
const localStorageStub = makeLocalStorageStub()
describe('installUnauthorizedFetchInterceptor', () => {
const originalFetch = globalThis.fetch
@@ -13,10 +32,13 @@ describe('installUnauthorizedFetchInterceptor', () => {
vi.resetModules()
mockLogoutUser.mockReset()
globalThis.fetch = vi.fn()
localStorageStub.clear()
vi.stubGlobal('localStorage', localStorageStub)
})
afterEach(() => {
globalThis.fetch = originalFetch
vi.unstubAllGlobals()
})
it('attempts refresh on 401, retries the original request on success', async () => {
@@ -181,4 +203,67 @@ describe('installUnauthorizedFetchInterceptor', () => {
expect(mockLogoutUser).not.toHaveBeenCalled()
expect(redirectSpy).not.toHaveBeenCalled()
})
it('sets lastRefreshAt in localStorage after a successful refresh', async () => {
const fetchMock = globalThis.fetch as unknown as ReturnType<typeof vi.fn>
fetchMock
.mockResolvedValueOnce({ status: 401 } as Response)
.mockResolvedValueOnce({ ok: true, status: 200 } as Response)
.mockResolvedValueOnce({ status: 200 } as Response)
window.history.pushState({}, '', '/parent')
const redirectSpy = vi.fn()
const {
installUnauthorizedFetchInterceptor,
setUnauthorizedRedirectHandlerForTests,
resetInterceptorStateForTests,
} = await import('../api')
resetInterceptorStateForTests()
setUnauthorizedRedirectHandlerForTests(redirectSpy)
installUnauthorizedFetchInterceptor()
await fetch('/api/user/profile')
const lastRefresh = localStorageStub.getItem('lastRefreshAt')
expect(lastRefresh).not.toBeNull()
expect(Number(lastRefresh)).toBeLessThanOrEqual(Date.now())
expect(mockLogoutUser).not.toHaveBeenCalled()
expect(redirectSpy).not.toHaveBeenCalled()
})
it('skips refresh call when another tab recently refreshed', async () => {
const fetchMock = globalThis.fetch as unknown as ReturnType<typeof vi.fn>
// Only original request and retry; refresh should be skipped due to cross-tab coordination
fetchMock
.mockResolvedValueOnce({ status: 401 } as Response)
.mockResolvedValueOnce({ status: 200, body: 'retried' } as unknown as Response)
window.history.pushState({}, '', '/parent')
const redirectSpy = vi.fn()
const {
installUnauthorizedFetchInterceptor,
setUnauthorizedRedirectHandlerForTests,
resetInterceptorStateForTests,
} = await import('../api')
resetInterceptorStateForTests()
setUnauthorizedRedirectHandlerForTests(redirectSpy)
installUnauthorizedFetchInterceptor()
// Simulate another tab having refreshed 1 second ago, after resetInterceptorStateForTests
localStorageStub.setItem('lastRefreshAt', String(Date.now() - 1000))
const result = await fetch('/api/user/profile')
// Should not call /api/auth/refresh; only original + retry
expect(fetchMock).toHaveBeenCalledTimes(2)
expect(fetchMock.mock.calls.map((c) => c[0])).toEqual([
'/api/user/profile',
'/api/user/profile',
])
expect(mockLogoutUser).not.toHaveBeenCalled()
expect(redirectSpy).not.toHaveBeenCalled()
expect((result as Response).status).toBe(200)
})
})
+12 -7
View File
@@ -5,15 +5,20 @@
"enabled": true,
"type": "local",
"command": [
"ssh",
"-T",
"devserver.lan",
"/home/ryan/gitea-mcp-wrapper.sh"
"docker",
"run",
"-i",
"--rm",
"-e",
"GITEA_ACCESS_TOKEN",
"-e",
"GITEA_HOST",
"docker.gitea.com/gitea-mcp-server"
],
"environment": {
"GITEA_HOST": "https://git.ryankegel.com"
},
"timeout": 30000
"GITEA_HOST": "https://git.ryankegel.com",
"GITEA_ACCESS_TOKEN": "{env:GITEA_ACCESS_TOKEN}"
}
}
}
}