diff --git a/.gitignore b/.gitignore index e70d85a..fb22a3f 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ backend/test-results/ **/.DS_Store frontend/cert.pem frontend/key.pem +tmp/ diff --git a/.opencode/skills/bugfix/SKILL.md b/.opencode/skills/bugfix/SKILL.md index c2e56c8..9272c9b 100644 --- a/.opencode/skills/bugfix/SKILL.md +++ b/.opencode/skills/bugfix/SKILL.md @@ -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. diff --git a/backend/api/auth_api.py b/backend/api/auth_api.py index c5d5924..91f5abf 100644 --- a/backend/api/auth_api.py +++ b/backend/api/auth_api.py @@ -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 diff --git a/backend/models/refresh_token.py b/backend/models/refresh_token.py index 9bba5ad..4c7723f 100644 --- a/backend/models/refresh_token.py +++ b/backend/models/refresh_token.py @@ -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'), ) diff --git a/backend/tests/test_auth_api.py b/backend/tests/test_auth_api.py index b972693..913c538 100644 --- a/backend/tests/test_auth_api.py +++ b/backend/tests/test_auth_api.py @@ -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 \ No newline at end of file + 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 diff --git a/frontend/src/common/__tests__/api.interceptor.spec.ts b/frontend/src/common/__tests__/api.interceptor.spec.ts index a2549dd..c15e28c 100644 --- a/frontend/src/common/__tests__/api.interceptor.spec.ts +++ b/frontend/src/common/__tests__/api.interceptor.spec.ts @@ -6,6 +6,25 @@ vi.mock('@/stores/auth', () => ({ logoutUser: () => mockLogoutUser(), })) +function makeLocalStorageStub() { + const store: Record = {} + 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 + 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 + // 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) + }) }) diff --git a/opencode.json b/opencode.json index 78657ad..24ebdce 100644 --- a/opencode.json +++ b/opencode.json @@ -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}" + } } } }