diff --git a/README.md b/README.md index d1cfb30..cae5278 100644 --- a/README.md +++ b/README.md @@ -275,6 +275,13 @@ curl -X POST http://localhost:8000/api/auth/refresh \\ Refresh-token reuse or a mismatched device ID returns `401` and revokes the token family. Signing out revokes the token family, while changing the password or completing a password reset revokes all sessions for the user. +Sign out with the access token in the bearer header: + +```sh +curl -X POST http://localhost:8000/api/auth/logout \\ + -H 'Authorization: Bearer YOUR_ACCESS_TOKEN' +``` + Submit a link using the returned access token: ```sh diff --git a/Security-audit.md b/Security-audit.md index e6997ad..7a99bc8 100644 --- a/Security-audit.md +++ b/Security-audit.md @@ -42,13 +42,15 @@ The application should remain behind the production reverse proxy, with real DNS ### SA-001: Logout uses non-standard token transport -**Severity:** High -**Evidence:** `POST /api/auth/logout` accepts `{"token": ...}` in the JSON request body, and the web frontend sends the access token this way. +**Severity:** High, remediated in current worktree +**Evidence before remediation:** `POST /api/auth/logout` accepted `{"token": ...}` in the JSON request body, and the web frontend sent the access token this way. **Impact:** Request bodies may be captured by debugging middleware, application logs, or monitoring systems. The endpoint also diverges from the bearer-header contract used elsewhere, increasing the chance of inconsistent token handling. -**Recommendation:** Make logout use `Authorization: Bearer ` and revoke the authenticated token or its family server-side. If a compatibility period is required, support both forms temporarily, prefer the header, and remove the body form after client migration. Add tests proving body-only tokens are rejected once compatibility is removed. +**Current state:** Logout requires `Authorization: Bearer `, rejects body-only tokens with `401`, and revokes the token family server-side. The web frontend and Firefox extension send the header; regression coverage verifies access and refresh tokens are invalid after logout. -**Priority:** High. +**Recommendation:** Keep logout header-only, retain family revocation, avoid logging authorization headers, and rotate legacy sessions issued before this change. + +**Priority:** Completed in code; legacy session rotation and log hygiene remain. ### SA-002: Raw infrastructure errors are returned to clients diff --git a/VIBE/CHAT_LOG.md b/VIBE/CHAT_LOG.md index 7256f5a..17de7ae 100644 --- a/VIBE/CHAT_LOG.md +++ b/VIBE/CHAT_LOG.md @@ -1,5 +1,11 @@ # Chat Log +### User +Remedy SA-001: Logout uses non-standard token transport. + +### Assistant outcome +Migrated logout to require `Authorization: Bearer `, updated the web frontend and Firefox extension, and added regression coverage proving body-only logout is rejected while header logout revokes the access token and refresh-token family. Updated SA-001, the checklist, and README examples. + ### User Perform a new security audit overwriting Security-audit.md with new and remaining issues. diff --git a/VIBE/PROMPTS.md b/VIBE/PROMPTS.md index 2c6ca63..1f4c18b 100644 --- a/VIBE/PROMPTS.md +++ b/VIBE/PROMPTS.md @@ -200,6 +200,7 @@ 194. Replace LINKLOG_TOKEN_EXPIRY_DAYS with LINKLOG_TOKEN_EXPIRY_MINUTES, add LINKLOG_REFRESH_TOKEN_EXPIRY_DAYS to production Compose, and add a CI configuration consistency test. 195. Perform a new security audit overwriting Security-audit.md with new and remaining issues. 195. Update SA-012 and README for the implemented refresh-token lifecycle, revocation behavior, and refresh endpoint. +196. Remedy SA-001: migrate logout from JSON token transport to the Authorization bearer header. ## Future entries diff --git a/XPI/unsigned/LinkLog-0.1.0.xpi b/XPI/unsigned/LinkLog-0.1.0.xpi index 6657fbf..a6196d2 100644 Binary files a/XPI/unsigned/LinkLog-0.1.0.xpi and b/XPI/unsigned/LinkLog-0.1.0.xpi differ diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index 7c88a88..d570bb7 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -3,7 +3,7 @@ from uuid import uuid4 -from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi import APIRouter, Depends, Header, HTTPException, Request from pydantic import BaseModel from backend.app.api.dependencies import get_current_user @@ -119,10 +119,12 @@ def reset_password_endpoint(payload: PasswordResetRequest): @router.post('/logout') -def logout(payload: dict): - token = payload.get('token') +def logout(authorization: str | None = Header(default=None)): + if not authorization or not authorization.startswith('Bearer '): + raise HTTPException(status_code=401, detail='Missing or invalid Authorization header') + token = authorization.replace('Bearer ', '', 1).strip() if not token: - raise HTTPException(status_code=400, detail='Token is required') + raise HTTPException(status_code=401, detail='Missing or invalid Authorization header') revoked = revoke_token(token) if not revoked: raise HTTPException(status_code=404, detail='Token not found or already revoked') diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index a3b72ac..c9dab5d 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -55,6 +55,17 @@ def test_login_returns_token(): assert client.get('/api/auth/me', params={'token': payload['access_token']}).status_code == 401 +def test_logout_requires_bearer_header_and_revokes_token_family(): + login = client.post('/api/auth/login', json={'email': 'alice@example.com', 'password': 'secret123'}).json() + token = login['access_token'] + headers = {'Authorization': f'Bearer {token}'} + assert client.post('/api/auth/logout', json={'token': token}).status_code == 401 + assert client.get('/api/auth/me', headers=headers).status_code == 200 + assert client.post('/api/auth/logout', headers=headers).status_code == 200 + assert client.get('/api/auth/me', headers=headers).status_code == 401 + assert client.post('/api/auth/refresh', json={'refresh_token': login['refresh_token'], 'device_id': login['device_id']}).status_code == 401 + + def test_login_rate_limit_locks_out_after_five_failures_and_resets_on_success(): email = f'unknown-{uuid4().hex}@example.com' for attempt in range(5): @@ -643,8 +654,7 @@ def test_only_link_owner_can_edit_link(): def test_logout_revokes_token_and_admin_can_list_plugins(): headers = login_headers() - token = headers['Authorization'].removeprefix('Bearer ') - assert client.post('/api/auth/logout', json={'token': token}).status_code == 200 + assert client.post('/api/auth/logout', headers=headers).status_code == 200 revoked_response = client.post('/api/links', headers=headers, json={ 'title': 'Should fail', diff --git a/frontend/static/logout.js b/frontend/static/logout.js index f1e6063..ac971f3 100644 --- a/frontend/static/logout.js +++ b/frontend/static/logout.js @@ -12,8 +12,7 @@ logoutButton.addEventListener('click', async () => { if (token) { await fetch('/api/auth/logout', { method: 'POST', - headers: {'Content-Type': 'application/json'}, - body: JSON.stringify({token}), + headers: {Authorization: `Bearer ${token}`}, }).catch(() => undefined); } diff --git a/webextension/options.js b/webextension/options.js index e5d927e..a5631bb 100644 --- a/webextension/options.js +++ b/webextension/options.js @@ -186,8 +186,7 @@ async function signOut() { if (sessionSettings.accessToken && settings.backendUrl && await hasBackendPermission(backendUrl)) { await fetch(`${backendUrl}/api/auth/logout`, { method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ token: sessionSettings.accessToken }), + headers: {Authorization: `Bearer ${sessionSettings.accessToken}`}, }).catch(() => undefined); } await clearSession();