OTP security hardened

This commit is contained in:
2026-08-26 18:24:02 +02:00
parent c3c3c8e1a6
commit 580c2a4257
13 changed files with 227 additions and 13 deletions
+15
View File
@@ -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()
+35 -3
View File
@@ -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'])
+12
View File
@@ -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);
''')
]
+31
View File
@@ -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)}'
+26
View File
@@ -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}'
+2 -2
View File
@@ -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()
+27 -1
View File
@@ -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']}"}