diff --git a/Security-audit.md b/Security-audit.md index 05bc593..ed35bfe 100644 --- a/Security-audit.md +++ b/Security-audit.md @@ -137,13 +137,17 @@ These findings are prioritized below. Severity describes the potential security ### SA-009: TOTP enrollment has no recovery codes or reset workflow -**Severity:** Medium -**Evidence:** `POST /api/user/otp/setup` returns the seed/provisioning URI and `POST /api/user/otp` requires a valid current OTP code to disable OTP. -**Impact:** A user who loses the authenticator device or seed can be locked out. Administrators have no documented recovery path that does not weaken authentication. Database readers can also use the plaintext seed as a second factor. +**Severity:** Medium, remediated in current worktree +**Evidence before remediation:** `POST /api/user/otp/setup` returned the seed/provisioning URI and `POST /api/user/otp` required a valid current OTP code to disable OTP. +**Impact:** A user who loses the authenticator device or seed could be locked out. Administrators had no documented recovery path that did not weaken authentication. -**Recommendation:** Generate one-time recovery codes during enrollment, display them once, hash them at rest, and invalidate each code on use. Require password reauthentication for disabling or replacing OTP. Add a controlled administrative recovery workflow with audit logging and notification. Avoid returning the seed after initial setup and never include it in profile responses. +**Current state:** OTP enrollment generates ten random recovery codes and returns them only in the enrollment response. The database stores only SHA-256 hashes, and each code is atomically marked used. Normal OTP disablement requires the current password and a valid TOTP code; `/api/user/otp/recover` requires the current password and a valid unused recovery code, then disables OTP and clears the seed. Profile responses do not include the seed or recovery codes. -**Priority:** Medium. +**Residual impact:** Recovery-code presentation is intentionally one-time; users who lose all codes can use the administrator-controlled OTP reset endpoint, which clears the seed and invalidates recovery codes. Recovery events should be added to the security audit log when SA-015 is addressed. + +**Recommendation:** Keep recovery codes out of logs and API responses after enrollment, notify users when OTP is disabled or recovered, and add a controlled administrative recovery workflow with audit logging and notification. + +**Priority:** Completed in code; operational recovery and audit logging remain. ### SA-010: Avatar validation trusts the client MIME type @@ -247,7 +251,7 @@ Before production exposure: - [ ] Configure HTTPS, HSTS, CSP, Referrer-Policy, frame protections, `nosniff`, and trusted hosts. - [ ] Define a restrictive CORS policy or leave CORS disabled. - [ ] Add global request-size limits and hardened image decoding/re-encoding. -- [ ] Add OTP recovery codes and a protected recovery workflow. +- [x] Add OTP recovery codes and a protected recovery workflow. - [ ] Remove or implement refresh-token behavior. - [ ] Add security audit events and centralized redacted logging. - [ ] Rotate all credentials and set a unique high-entropy production secret. diff --git a/VIBE/CHAT_LOG.md b/VIBE/CHAT_LOG.md index b97c630..e1c68dd 100644 --- a/VIBE/CHAT_LOG.md +++ b/VIBE/CHAT_LOG.md @@ -6,12 +6,24 @@ Address SA-007 and use linklog.example.com as default LINKLOG_PUBLIC_URL. ### Assistant outcome Hardened the production Compose deployment by removing direct host port publication, keeping the app on the external Traefik network, and using `linklog.example.com` as the default public hostname. Added `docker-compose.local.yml` for explicit local direct-port development, updated `.env.example`, backend defaults, README deployment instructions, and SA-007 status. +### User +Fix SA-009: TOTP enrollment has no recovery codes or reset workflow. + +### Assistant outcome +Implemented ten one-time hashed TOTP recovery codes, a password-protected `/api/user/otp/recover` workflow, password reauthentication for normal OTP disablement, one-time code consumption, and profile-page controls for saving and using recovery codes. Updated SA-009 and the deployment checklist. + ### User Continue SA-06: Store session credentials in the narrowest available extension storage, minimize token lifetime, support refresh-token rotation, and clear all session material on logout or token invalidation. ### Assistant outcome Implemented SA-006 session hardening: access tokens now default to 15 minutes, refresh tokens are hashed and persisted separately with device and family binding, rotation revokes prior family state and detects reuse, and password reset revokes sessions. The extension uses Firefox session storage for credentials, refreshes on 401, clears session and legacy local token keys on invalidation/logout, requests only the configured backend origin, and uses a self-only extension-page CSP. +### User +Also allow an admin to reset (disable) OTP for any user. + +### Assistant outcome +Added an administrator-only OTP reset endpoint and admin UI action. Resetting a user clears the OTP seed, disables OTP, and deletes all recovery codes. The operation never returns OTP secrets or recovery material and is covered by authorization, missing-user, state, and recovery-code invalidation tests. + ### User Implement SA-006 permission changes: remove `` and unnecessary `tabs`, request exact access to the configured self-hosted backend, and keep page capture behind `activeTab`. diff --git a/VIBE/PROMPTS.md b/VIBE/PROMPTS.md index 99b93ef..6510d2f 100644 --- a/VIBE/PROMPTS.md +++ b/VIBE/PROMPTS.md @@ -184,6 +184,8 @@ 180. Implement SA-006: remove broad Firefox extension host access and unnecessary tabs permission, request exact configured backend origin access, and keep page capture behind activeTab. 181. Continue to document every prompt and chat in the VIBE directory. 182. Continue SA-006: store session credentials in the narrowest available extension storage, minimize token lifetime, support refresh-token rotation, and clear all session material on logout or token invalidation. +183. Also allow an admin to reset (disable) OTP for any user. +183. Fix SA-009: TOTP enrollment has no recovery codes or reset workflow. 183. Address SA-007 and use linklog.example.com as the default LINKLOG_PUBLIC_URL. ## Future entries diff --git a/backend/app/api/admin.py b/backend/app/api/admin.py index 4d717b7..75e4223 100644 --- a/backend/app/api/admin.py +++ b/backend/app/api/admin.py @@ -94,6 +94,21 @@ def list_users(_: dict = Depends(require_admin)): return [public_user(row) for row in rows] +@router.post('/users/{user_id}/otp/reset') +def reset_user_otp(user_id: str, _: dict = Depends(require_admin)): + with get_connection() as conn: + target = conn.execute('SELECT id FROM users WHERE id = ?', (user_id,)).fetchone() + if target is None: + raise HTTPException(status_code=404, detail='User not found') + conn.execute( + 'UPDATE users SET otp_enabled = 0, otp_secret = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?', + (user_id,), + ) + conn.execute('DELETE FROM otp_recovery_codes WHERE user_id = ?', (user_id,)) + conn.commit() + return {'status': 'otp_reset', 'enabled': False, 'user_id': user_id} + + @router.post('/users', status_code=201) def create_user(payload: AdminUserCreate, _: dict = Depends(require_admin)): username = payload.username.strip() diff --git a/backend/app/api/user_config.py b/backend/app/api/user_config.py index 7f41d4a..5c18690 100644 --- a/backend/app/api/user_config.py +++ b/backend/app/api/user_config.py @@ -11,7 +11,7 @@ from pydantic import BaseModel from backend.app.api.dependencies import get_current_user from backend.app.database import AVATARS_DIR, get_connection, hash_password, verify_password from backend.app.services.link_service import create_label, delete_label, list_user_labels, update_label -from backend.app.services.otp_service import create_secret, provisioning_uri, verify_code +from backend.app.services.otp_service import consume_recovery_code, create_recovery_codes, create_secret, provisioning_uri, verify_code from backend.app.services.email_addresses import add_user_email_address, create_email_verification, list_user_email_addresses from backend.app.services.email_service import send_verification_email, smtp_configured from backend.app.core.config import settings @@ -32,12 +32,19 @@ class PasswordUpdate(BaseModel): class OtpUpdate(BaseModel): action: str code: str | None = None + current_password: str | None = None + recovery_code: str | None = None class AdditionalEmail(BaseModel): email: str +class OtpRecovery(BaseModel): + current_password: str + recovery_code: str + + class UserPluginConfigUpdate(BaseModel): instance: str | None = None @@ -119,14 +126,24 @@ def setup_otp(user: dict = Depends(get_current_user)): with get_connection() as conn: conn.execute('UPDATE users SET otp_secret = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', (encrypt_secret(secret), user['id'])) conn.commit() - return {'secret': secret, 'otpauth_url': provisioning_uri(secret, user['username'])} + return { + 'secret': secret, + 'otpauth_url': provisioning_uri(secret, user['username']), + 'recovery_codes': create_recovery_codes(user['id']), + } @router.post('/otp') def update_otp(payload: OtpUpdate, user: dict = Depends(get_current_user)): if payload.action not in {'enable', 'disable'}: raise HTTPException(status_code=422, detail='OTP action must be enable or disable') - if not verify_code(decrypt_secret(user['otp_secret']), payload.code): + if payload.action == 'disable' and not payload.current_password: + raise HTTPException(status_code=400, detail='Current password is required to disable one-time password') + if payload.action == 'disable' and not verify_password(payload.current_password, user['password_hash']): + raise HTTPException(status_code=400, detail='Current password is incorrect') + valid_code = verify_code(decrypt_secret(user['otp_secret']), payload.code) + valid_recovery_code = payload.action == 'disable' and payload.recovery_code and consume_recovery_code(user['id'], payload.recovery_code) + if not valid_code and not valid_recovery_code: raise HTTPException(status_code=400, detail='Invalid one-time password') with get_connection() as conn: if payload.action == 'enable': @@ -137,6 +154,21 @@ def update_otp(payload: OtpUpdate, user: dict = Depends(get_current_user)): return {'status': 'updated', 'enabled': payload.action == 'enable'} +@router.post('/otp/recover') +def recover_otp(payload: OtpRecovery, user: dict = Depends(get_current_user)): + if not verify_password(payload.current_password, user['password_hash']): + raise HTTPException(status_code=400, detail='Current password is incorrect') + if not consume_recovery_code(user['id'], payload.recovery_code): + raise HTTPException(status_code=400, detail='Recovery code is invalid or already used') + with get_connection() as conn: + conn.execute( + 'UPDATE users SET otp_enabled = 0, otp_secret = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?', + (user['id'],), + ) + conn.commit() + return {'status': 'otp_recovered', 'enabled': False} + + @router.get('/emails') def get_additional_emails(user: dict = Depends(get_current_user)): return [{'email': user['email'], 'verified': bool(user['email_verified']), 'primary': True}] + list_user_email_addresses(user['id']) diff --git a/backend/app/database.py b/backend/app/database.py index db4e7f6..e895173 100644 --- a/backend/app/database.py +++ b/backend/app/database.py @@ -232,6 +232,18 @@ ALTER TABLE tokens ADD COLUMN device_id TEXT; ALTER TABLE tokens ADD COLUMN token_family_id TEXT; CREATE INDEX IF NOT EXISTS idx_tokens_device_id ON tokens(device_id); CREATE INDEX IF NOT EXISTS idx_tokens_family_id ON tokens(token_family_id); +'''), + (16, ''' +CREATE TABLE IF NOT EXISTS otp_recovery_codes ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL, + code_hash TEXT NOT NULL UNIQUE, + used INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + used_at TEXT, + FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_otp_recovery_codes_user_id ON otp_recovery_codes(user_id); ''') ] diff --git a/backend/app/services/otp_service.py b/backend/app/services/otp_service.py index f969117..a462e98 100644 --- a/backend/app/services/otp_service.py +++ b/backend/app/services/otp_service.py @@ -7,12 +7,43 @@ import hmac import secrets import time from urllib.parse import quote +from uuid import uuid4 + +from backend.app.database import get_connection def create_secret() -> str: return base64.b32encode(secrets.token_bytes(20)).decode('ascii').rstrip('=') +def create_recovery_codes(user_id: str, count: int = 10) -> list[str]: + codes = [secrets.token_urlsafe(9) for _ in range(count)] + with get_connection() as conn: + conn.execute('DELETE FROM otp_recovery_codes WHERE user_id = ?', (user_id,)) + conn.executemany( + 'INSERT INTO otp_recovery_codes (id, user_id, code_hash) VALUES (?, ?, ?)', + [(str(uuid4()), user_id, hash_recovery_code(code)) for code in codes], + ) + conn.commit() + return codes + + +def hash_recovery_code(code: str) -> str: + return hashlib.sha256(code.strip().encode('utf-8')).hexdigest() + + +def consume_recovery_code(user_id: str, code: str) -> bool: + with get_connection() as conn: + cursor = conn.execute( + '''UPDATE otp_recovery_codes + SET used = 1, used_at = CURRENT_TIMESTAMP + WHERE user_id = ? AND code_hash = ? AND used = 0''', + (user_id, hash_recovery_code(code)), + ) + conn.commit() + return cursor.rowcount == 1 + + def provisioning_uri(secret: str, username: str, issuer: str = 'LinkLog') -> str: return f'otpauth://totp/{quote(issuer)}:{quote(username)}?secret={secret}&issuer={quote(issuer)}' diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 34348c4..aa570c6 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -239,6 +239,32 @@ def test_admin_can_add_list_and_remove_users(): assert client.put('/api/admin/users/user-1', headers=headers, json={'is_admin': False}).status_code == 400 +def test_admin_can_reset_another_users_otp(): + admin_headers = login_headers() + user_login = client.post('/api/auth/login', json={ + 'email': 'bob@example.com', + 'password': 'secret123', + }).json() + user_headers = {'Authorization': f"Bearer {user_login['access_token']}"} + setup = client.post('/api/user/otp/setup', headers=user_headers) + assert setup.status_code == 200 + secret = setup.json()['secret'] + recovery_code = setup.json()['recovery_codes'][0] + assert client.post('/api/user/otp', headers=user_headers, json={ + 'action': 'enable', 'code': current_code(secret), + }).status_code == 200 + + assert client.post('/api/admin/users/user-2/otp/reset', headers=admin_headers).json() == { + 'status': 'otp_reset', 'enabled': False, 'user_id': 'user-2', + } + assert client.get('/api/user/otp', headers=user_headers).json() == {'enabled': False} + assert client.post('/api/user/otp/recover', headers=user_headers, json={ + 'current_password': 'secret123', 'recovery_code': recovery_code, + }).status_code == 400 + assert client.post('/api/admin/users/user-2/otp/reset', headers=login_headers('bob')).status_code == 403 + assert client.post('/api/admin/users/missing-user/otp/reset', headers=admin_headers).status_code == 404 + + def test_new_user_must_verify_email_before_login(): headers = login_headers() username = f'unverified-{uuid4().hex}' diff --git a/backend/tests/test_database.py b/backend/tests/test_database.py index eb9b84b..1d98e5b 100644 --- a/backend/tests/test_database.py +++ b/backend/tests/test_database.py @@ -10,7 +10,7 @@ def test_database_migrations_are_versioned_and_idempotent(): connection = sqlite3.connect(':memory:') apply_migrations(connection) - assert get_schema_version(connection) == 15 + assert get_schema_version(connection) == 16 tables = { row[0] for row in connection.execute( @@ -27,6 +27,6 @@ def test_database_migrations_are_versioned_and_idempotent(): assert set(DEFAULT_TAGS) <= seeded_tags apply_migrations(connection) - assert get_schema_version(connection) == 15 + assert get_schema_version(connection) == 16 connection.close() \ No newline at end of file diff --git a/backend/tests/test_user_config.py b/backend/tests/test_user_config.py index 9f7f6d0..a311ed0 100644 --- a/backend/tests/test_user_config.py +++ b/backend/tests/test_user_config.py @@ -89,6 +89,8 @@ def test_user_can_enable_and_use_otp(): assert setup.status_code == 200 secret = setup.json()['secret'] assert setup.json()['otpauth_url'].startswith('otpauth://totp/') + recovery_codes = setup.json()['recovery_codes'] + assert len(recovery_codes) == 10 enabled = client.post('/api/user/otp', headers=headers, json={ 'action': 'enable', 'code': current_code(secret), @@ -103,12 +105,36 @@ def test_user_can_enable_and_use_otp(): assert otp_login.status_code == 200 disabled = client.post('/api/user/otp', headers=headers, json={ - 'action': 'disable', 'code': current_code(secret), + 'action': 'disable', 'code': current_code(secret), 'current_password': 'secret123', }) assert disabled.status_code == 200 assert disabled.json()['enabled'] is False +def test_otp_recovery_code_requires_password_and_is_single_use(): + login = client.post('/api/auth/login', json={'email': 'alice@example.com', 'password': 'secret123'}).json() + headers = {'Authorization': f"Bearer {login['access_token']}"} + setup = client.post('/api/user/otp/setup', headers=headers) + secret = setup.json()['secret'] + recovery_code = setup.json()['recovery_codes'][0] + assert client.post('/api/user/otp', headers=headers, json={ + 'action': 'enable', 'code': current_code(secret), + }).status_code == 200 + + rejected = client.post('/api/user/otp/recover', headers=headers, json={ + 'current_password': 'wrong-password', 'recovery_code': recovery_code, + }) + assert rejected.status_code == 400 + recovered = client.post('/api/user/otp/recover', headers=headers, json={ + 'current_password': 'secret123', 'recovery_code': recovery_code, + }) + assert recovered.status_code == 200 + reused = client.post('/api/user/otp/recover', headers=headers, json={ + 'current_password': 'secret123', 'recovery_code': recovery_code, + }) + assert reused.status_code == 400 + + def test_verified_alternative_can_become_primary(): login = client.post('/api/auth/login', json={'email': 'alice@example.com', 'password': 'secret123'}).json() headers = {'Authorization': f"Bearer {login['access_token']}"} diff --git a/frontend/static/admin.js b/frontend/static/admin.js index 62d4aa1..a23d6c5 100644 --- a/frontend/static/admin.js +++ b/frontend/static/admin.js @@ -152,6 +152,11 @@ function renderUsers(users) { privilegeLabel.append(privilegeCheckbox, document.createTextNode(' Administrator')); row.append(label, privilegeLabel); if (!isCurrentUser) { + const otpButton = document.createElement('button'); + otpButton.type = 'button'; + otpButton.textContent = 'Reset OTP'; + otpButton.addEventListener('click', () => resetUserOtp(user, otpButton)); + row.append(otpButton); const button = document.createElement('button'); button.type = 'button'; button.className = 'danger-button'; @@ -163,6 +168,27 @@ function renderUsers(users) { })); } +async function resetUserOtp(user, button) { + if (!window.confirm(`Disable OTP for ${user.username}?`)) return; + button.disabled = true; + const status = document.querySelector('#user-status'); + try { + const response = await fetch(`/api/admin/users/${encodeURIComponent(user.id)}/otp/reset`, { + method: 'POST', + headers: authHeaders(), + }); + if (!response.ok) { + throw new Error(await responseError(response, `Request failed (${response.status})`)); + } + status.textContent = `OTP disabled for ${user.username}.`; + status.style.color = '#94e2d5'; + } catch (error) { + status.textContent = `Could not reset OTP for ${user.username}: ${error.message}`; + status.style.color = '#f38ba8'; + button.disabled = false; + } +} + async function loadUsers() { const response = await fetch('/api/admin/users', {headers: authHeaders()}); if (!response.ok) throw new Error('Could not load users'); diff --git a/frontend/static/profile.js b/frontend/static/profile.js index cdeb1c5..2bfffd8 100644 --- a/frontend/static/profile.js +++ b/frontend/static/profile.js @@ -12,11 +12,13 @@ const mastodonConnectButton = document.querySelector('#mastodon-connect'); const otpSetupButton = document.querySelector('#otp-setup'); const otpEnableButton = document.querySelector('#otp-enable'); const otpDisableButton = document.querySelector('#otp-disable'); +const otpRecoverButton = document.querySelector('#otp-recover'); const otpProvisioning = document.querySelector('#otp-provisioning'); const otpDisabled = document.querySelector('#otp-disabled'); const otpEnabled = document.querySelector('#otp-enabled'); const otpSecret = document.querySelector('#otp-secret'); const otpUri = document.querySelector('#otp-uri'); +const otpRecoveryCodes = document.querySelector('#otp-recovery-codes'); const otpStatus = document.querySelector('#otp-status'); const emailAddressList = document.querySelector('#email-address-list'); const additionalEmailForm = document.querySelector('#additional-email-form'); @@ -131,6 +133,7 @@ otpSetupButton.addEventListener('click', async () => { } otpSecret.textContent = result.secret; otpUri.href = result.otpauth_url; + otpRecoveryCodes.textContent = result.recovery_codes.join('\n'); otpProvisioning.classList.remove('hidden'); setOtpStatus('Enter a code from your authenticator app to confirm setup.'); }); @@ -153,8 +156,9 @@ otpEnableButton.addEventListener('click', async () => { otpDisableButton.addEventListener('click', async () => { const code = document.querySelector('#otp-disable-code').value.trim(); + const currentPassword = document.querySelector('#otp-current-password').value; const response = await fetch('/api/user/otp', { - method: 'POST', headers: authHeaders(true), body: JSON.stringify({action: 'disable', code}), + method: 'POST', headers: authHeaders(true), body: JSON.stringify({action: 'disable', code, current_password: currentPassword}), }); const result = await response.json(); if (!response.ok) { @@ -167,6 +171,24 @@ otpDisableButton.addEventListener('click', async () => { setOtpStatus('One-time password disabled.'); }); +otpRecoverButton.addEventListener('click', async () => { + const currentPassword = document.querySelector('#otp-current-password').value; + const recoveryCode = document.querySelector('#otp-recovery-code').value.trim(); + const response = await fetch('/api/user/otp/recover', { + method: 'POST', headers: authHeaders(true), body: JSON.stringify({current_password: currentPassword, recovery_code: recoveryCode}), + }); + const result = await response.json(); + if (!response.ok) { + setOtpStatus(result.detail || 'Could not recover one-time password access.', true); + return; + } + otpDisabled.classList.remove('hidden'); + otpEnabled.classList.add('hidden'); + document.querySelector('#otp-current-password').value = ''; + document.querySelector('#otp-recovery-code').value = ''; + setOtpStatus('One-time password access recovered.'); +}); + async function loadProfile() { const response = await fetch('/api/user/me', {headers: authHeaders()}); if (!response.ok) throw new Error('Could not load profile'); diff --git a/frontend/templates/user_profile.html b/frontend/templates/user_profile.html index 3604004..1915670 100644 --- a/frontend/templates/user_profile.html +++ b/frontend/templates/user_profile.html @@ -108,13 +108,19 @@ +

Save these recovery codes in a secure place. They are shown only once:

+