05/17 enhance codes

This commit is contained in:
Nguyen Ngo
2026-05-17 18:29:13 -04:00
parent eee5f62689
commit afc3250c48
6 changed files with 246 additions and 52 deletions
+5 -2
View File
@@ -104,6 +104,7 @@ def create_app(config_name: str = 'development') -> Flask:
from .models.shared_item import SharedItem from .models.shared_item import SharedItem
from .models.emergency_access import EmergencyAccess from .models.emergency_access import EmergencyAccess
from .models.audit_log import AuditLog from .models.audit_log import AuditLog
from .models.recovery_challenge import RecoveryChallenge
@login_manager.user_loader @login_manager.user_loader
def load_user(user_id): def load_user(user_id):
@@ -188,15 +189,17 @@ def create_app(config_name: str = 'development') -> Flask:
with app.app_context(): with app.app_context():
try: try:
from app.models.token_blacklist import TokenBlacklist from app.models.token_blacklist import TokenBlacklist
from app.models.recovery_challenge import RecoveryChallenge
TokenBlacklist.cleanup_expired() TokenBlacklist.cleanup_expired()
RecoveryChallenge.cleanup_expired()
import logging import logging
logging.getLogger(__name__).debug( logging.getLogger(__name__).debug(
'[PassKeeper] token_blacklist cleanup completed' '[PassKeeper] token_blacklist + recovery_challenges cleanup completed'
) )
except Exception as exc: # pragma: no cover except Exception as exc: # pragma: no cover
import logging import logging
logging.getLogger(__name__).warning( logging.getLogger(__name__).warning(
'[PassKeeper] token_blacklist cleanup failed: %s', exc '[PassKeeper] cleanup job failed: %s', exc
) )
scheduler = BackgroundScheduler(daemon=True) scheduler = BackgroundScheduler(daemon=True)
+91
View File
@@ -0,0 +1,91 @@
from datetime import datetime, timezone, timedelta
from sqlalchemy.dialects.mysql import INTEGER
from app import db
# Challenge TTL: the client has 15 minutes from /recovery/data to complete recovery.
CHALLENGE_TTL_MINUTES = 15
class RecoveryChallenge(db.Model):
"""
Server-side state for the account-recovery challenge-response flow.
Replaces the previous flask.session-based storage, which was not safe in
multi-worker Gunicorn deployments (each worker has its own in-process
session store, so a challenge written by worker A is invisible to worker B).
A row is created (or replaced) by GET /recovery/data and consumed
(deleted) exactly once by POST /recover or GET /recovery/items.
Rows older than CHALLENGE_TTL_MINUTES are ignored and cleaned up by the
same APScheduler job that handles token_blacklist.
Columns
-------
user_id FK → users.id (CASCADE DELETE)
nonce Random 64-hex-char string issued to the client.
expected_proof HMAC-SHA256(key=enc_key_salt, msg=nonce), hex-encoded.
Precomputed at challenge creation so the server never needs
to touch enc_key_salt again during verification.
expires_at Absolute UTC timestamp after which this challenge is void.
"""
__tablename__ = 'recovery_challenges'
id = db.Column(INTEGER(unsigned=True), autoincrement=True, primary_key=True)
user_id = db.Column(
INTEGER(unsigned=True),
db.ForeignKey('users.id', ondelete='CASCADE'),
nullable=False,
unique=True, # one active challenge per user at a time
index=True,
)
nonce = db.Column(db.String(64), nullable=False)
expected_proof = db.Column(db.String(64), nullable=False)
expires_at = db.Column(db.DateTime, nullable=False)
@classmethod
def create(cls, user_id: int, nonce: str, expected_proof: str) -> 'RecoveryChallenge':
"""
Upsert: delete any existing challenge for this user, then create a fresh one.
Caller must call db.session.commit() after this.
"""
# Remove stale challenge so the UNIQUE constraint never fires on re-issue.
cls.query.filter_by(user_id=user_id).delete()
challenge = cls(
user_id=user_id,
nonce=nonce,
expected_proof=expected_proof,
expires_at=datetime.now(timezone.utc).replace(tzinfo=None)
+ timedelta(minutes=CHALLENGE_TTL_MINUTES),
)
db.session.add(challenge)
return challenge
@classmethod
def consume(cls, user_id: int) -> 'RecoveryChallenge | None':
"""
Fetch and delete the challenge for user_id in one operation.
Returns the challenge if it exists and has not expired, otherwise None.
Caller must call db.session.commit() after this.
"""
challenge = cls.query.filter_by(user_id=user_id).first()
if not challenge:
return None
if challenge.expires_at < datetime.now(timezone.utc).replace(tzinfo=None):
cls.query.filter_by(user_id=user_id).delete()
return None
# Delete before returning — one-time use.
cls.query.filter_by(user_id=user_id).delete()
return challenge
@classmethod
def cleanup_expired(cls) -> None:
"""Delete expired challenges — called by the APScheduler cleanup job."""
cls.query.filter(
cls.expires_at <= datetime.now(timezone.utc).replace(tzinfo=None)
).delete()
db.session.commit()
def __repr__(self):
return f'<RecoveryChallenge user_id={self.user_id} expires={self.expires_at}>'
+45 -34
View File
@@ -1,7 +1,7 @@
import re import re
import time import time
from flask import Blueprint, request, jsonify, g, session from flask import Blueprint, request, jsonify, g
from app import db, limiter, client_ip from app import db, limiter, client_ip
from app.models.user import User from app.models.user import User
from app.models.audit_log import AuditLog from app.models.audit_log import AuditLog
@@ -738,10 +738,13 @@ def recover_account():
4. Client derives new credentials and re-encrypts all vault items. 4. Client derives new credentials and re-encrypts all vault items.
5. Client POSTs everything here in one atomic payload. 5. Client POSTs everything here in one atomic payload.
The server validates recovery_proof against the value precomputed during The server validates recovery_proof against the value stored in the DB
/recovery/data — enc_key_salt is never sent in plaintext. during /recovery/data — enc_key_salt is never sent in plaintext.
The nonce is consumed on first use to prevent replay. The challenge row is consumed (deleted) on first use to prevent replay.
Challenge state is stored in the database, not the Flask session, so the
flow works correctly across all Gunicorn workers.
""" """
from app.models.recovery_challenge import RecoveryChallenge
data = request.get_json(silent=True) or {} data = request.get_json(silent=True) or {}
email = (data.get('email') or '').strip().lower() email = (data.get('email') or '').strip().lower()
new_auth_hash = data.get('new_auth_hash', '') new_auth_hash = data.get('new_auth_hash', '')
@@ -754,18 +757,17 @@ def recover_account():
time.sleep(0.1) # timing mitigation time.sleep(0.1) # timing mitigation
# Validate session binding.
expected_proof = session.get('recovery_expected_proof', '')
session_user_id = session.get('recovery_user_id')
if not expected_proof or not session_user_id:
return jsonify({'error': 'No active recovery challenge. Call /recovery/data first.'}), 400
user = User.query.filter_by(email=email).first() user = User.query.filter_by(email=email).first()
if not user or not user.recovery_enc_salt or user.id != session_user_id: if not user or not user.recovery_enc_salt:
return jsonify({'error': 'No recovery code found for this account'}), 404 return jsonify({'error': 'No recovery code found for this account'}), 404
if not verify_recovery_proof(expected_proof, client_proof): # Consume the challenge — atomic read-and-delete from the DB.
# consume() returns None if the challenge is missing or expired.
challenge = RecoveryChallenge.consume(user.id)
if not challenge:
return jsonify({'error': 'No active recovery challenge. Call /recovery/data first.'}), 400
if not verify_recovery_proof(challenge.expected_proof, client_proof):
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
action='auth.recovery_failed', action='auth.recovery_failed',
@@ -777,10 +779,6 @@ def recover_account():
db.session.commit() db.session.commit()
return jsonify({'error': 'Invalid recovery code'}), 401 return jsonify({'error': 'Invalid recovery code'}), 401
# Consume the nonce — one-time use only.
session.pop('recovery_nonce', None)
session.pop('recovery_expected_proof', None)
session.pop('recovery_user_id', None)
try: try:
from app.models.vault_item import VaultItem from app.models.vault_item import VaultItem
@@ -847,12 +845,17 @@ def recovery_data():
The nonce is used for the HMAC-SHA256 challenge-response proof: The nonce is used for the HMAC-SHA256 challenge-response proof:
- Client decrypts recovery_enc_salt → gets enc_key_salt bytes. - Client decrypts recovery_enc_salt → gets enc_key_salt bytes.
- Client computes: proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce) - Client computes: proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce)
- Server stores expected proof in server-side session at challenge time, - Server stores expected proof in the DB (recovery_challenges table),
verifying it on /recover and /recovery/items without ever receiving verifying it on /recover and /recovery/items without ever receiving
enc_key_salt in plaintext. enc_key_salt in plaintext.
Returns 404 if no recovery code is configured (prevents user enumeration). Returns 404 if no recovery code is configured (prevents user enumeration).
The challenge is stored in the database (not the Flask session cookie) so
it works correctly across all Gunicorn workers.
""" """
import hashlib, hmac as _hmac
from app.models.recovery_challenge import RecoveryChallenge
email = (request.args.get('email') or '').strip().lower() email = (request.args.get('email') or '').strip().lower()
if not email: if not email:
return jsonify({'error': 'email is required'}), 400 return jsonify({'error': 'email is required'}), 400
@@ -865,7 +868,6 @@ def recovery_data():
# enc_key_salt. The client must return HMAC-SHA256(enc_key_salt, nonce). # enc_key_salt. The client must return HMAC-SHA256(enc_key_salt, nonce).
# This proves it decrypted the recovery blob correctly without sending # This proves it decrypted the recovery blob correctly without sending
# enc_key_salt in plaintext. # enc_key_salt in plaintext.
import hashlib, hmac as _hmac
nonce = generate_recovery_nonce() nonce = generate_recovery_nonce()
expected_proof = _hmac.new( expected_proof = _hmac.new(
user.enc_key_salt.encode(), user.enc_key_salt.encode(),
@@ -873,10 +875,15 @@ def recovery_data():
hashlib.sha256, hashlib.sha256,
).hexdigest() ).hexdigest()
# Store expected proof and bind it to the user — consumed on first use. # Persist challenge in the DB — safe across all Gunicorn workers.
session['recovery_nonce'] = nonce # RecoveryChallenge.create() deletes any previous challenge for this user
session['recovery_expected_proof'] = expected_proof # before inserting, so a re-issued challenge always starts fresh.
session['recovery_user_id'] = user.id RecoveryChallenge.create(
user_id=user.id,
nonce=nonce,
expected_proof=expected_proof,
)
db.session.commit()
return jsonify({ return jsonify({
'enc_key_salt': user.enc_key_salt, 'enc_key_salt': user.enc_key_salt,
@@ -895,28 +902,32 @@ def recovery_items():
Requires X-Recovery-Proof header containing the HMAC-SHA256 proof: Requires X-Recovery-Proof header containing the HMAC-SHA256 proof:
proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce_from_recovery_data) proof = HMAC-SHA256(key=enc_key_salt_bytes, msg=nonce_from_recovery_data)
The server validates the proof against the value precomputed and stored in The server validates the proof against the value stored in the DB
flask.session during /recovery/data — enc_key_salt is never sent in plaintext. during /recovery/data — enc_key_salt is never sent in plaintext.
The challenge row is NOT consumed here; it is consumed by the final
POST /recover call so the client can call this endpoint to preview
items before committing the full re-encryption.
Items are returned as encrypted ciphertext blobs only. Items are returned as encrypted ciphertext blobs only.
""" """
from app.models.recovery_challenge import RecoveryChallenge
email = (request.args.get('email') or '').strip().lower() email = (request.args.get('email') or '').strip().lower()
client_proof = request.headers.get('X-Recovery-Proof', '').strip() client_proof = request.headers.get('X-Recovery-Proof', '').strip()
if not email or not client_proof: if not email or not client_proof:
return jsonify({'error': 'email and X-Recovery-Proof header are required'}), 400 return jsonify({'error': 'email and X-Recovery-Proof header are required'}), 400
# Validate session binding: proof must match what was issued to this session.
expected_proof = session.get('recovery_expected_proof', '')
session_user_id = session.get('recovery_user_id')
if not expected_proof or not session_user_id:
return jsonify({'error': 'No active recovery challenge. Call /recovery/data first.'}), 400
user = User.query.filter_by(email=email).first() user = User.query.filter_by(email=email).first()
if not user or not user.recovery_enc_salt or user.id != session_user_id: if not user or not user.recovery_enc_salt:
return jsonify({'error': 'No recovery data found'}), 404 return jsonify({'error': 'No recovery data found'}), 404
if not verify_recovery_proof(expected_proof, client_proof): # Validate against the DB-stored challenge — safe across all workers.
challenge = RecoveryChallenge.query.filter_by(user_id=user.id).first()
from datetime import datetime, timezone
if not challenge or challenge.expires_at < datetime.now(timezone.utc).replace(tzinfo=None):
return jsonify({'error': 'No active recovery challenge. Call /recovery/data first.'}), 400
if not verify_recovery_proof(challenge.expected_proof, client_proof):
AuditLog.log( AuditLog.log(
user_id=user.id, user_id=user.id,
action='auth.recovery_items_denied', action='auth.recovery_items_denied',
+20 -9
View File
@@ -129,18 +129,29 @@ def blacklist_token(token: str, token_type: str) -> None:
if not jti: if not jti:
return return
exp = payload.get('exp') exp = payload.get('exp')
expires_at = datetime.fromtimestamp(exp, tz=timezone.utc).replace(tzinfo=None) if exp else datetime.now(timezone.utc).replace(tzinfo=None) + timedelta(days=7) expires_at = (
datetime.fromtimestamp(exp, tz=timezone.utc).replace(tzinfo=None)
if exp
else datetime.now(timezone.utc).replace(tzinfo=None) + timedelta(days=7)
)
from app.models.token_blacklist import TokenBlacklist from app.models.token_blacklist import TokenBlacklist
from app import db from app import db
# Avoid duplicate if already blacklisted from sqlalchemy.exc import IntegrityError
if not TokenBlacklist.query.filter_by(jti=jti).first(): # INSERT directly — no SELECT-before-INSERT race.
entry = TokenBlacklist( # Two concurrent logouts of the same token would both try to insert,
jti=jti, # but the UNIQUE constraint on jti makes exactly one succeed.
user_id=int(payload.get('sub', 0)), # We catch IntegrityError and roll back gracefully; the token is
expires_at=expires_at, # already blacklisted so the outcome is correct either way.
) entry = TokenBlacklist(
db.session.add(entry) jti=jti,
user_id=int(payload.get('sub', 0)),
expires_at=expires_at,
)
db.session.add(entry)
try:
db.session.commit() db.session.commit()
except IntegrityError:
db.session.rollback() # already blacklisted — safe to ignore
# Cleanup is handled by the APScheduler background job in create_app(), # Cleanup is handled by the APScheduler background job in create_app(),
# not here — keeps the logout/refresh hot path free of extra DB writes. # not here — keeps the logout/refresh hot path free of extra DB writes.
except Exception: except Exception:
+22 -7
View File
@@ -235,6 +235,13 @@ function isMatch(item) {
// ── API helpers ─────────────────────────────────────────────────────────────── // ── API helpers ───────────────────────────────────────────────────────────────
// Singleton promise for the in-flight token refresh.
// When a 401 triggers a refresh, all concurrent requests that also receive a
// 401 await this same promise instead of starting their own — preventing the
// second refresh from using an already-rotated (and therefore blacklisted)
// refresh token, which would cause an unexpected sign-out.
let _refreshPromise = null;
async function apiFetch(path, options = {}) { async function apiFetch(path, options = {}) {
const { access_token } = await chrome.storage.session.get("access_token"); const { access_token } = await chrome.storage.session.get("access_token");
const headers = { const headers = {
@@ -246,7 +253,13 @@ async function apiFetch(path, options = {}) {
let res = await fetch(`${API_BASE}${path}`, { ...options, headers }); let res = await fetch(`${API_BASE}${path}`, { ...options, headers });
if (res.status === 401) { if (res.status === 401) {
const refreshed = await tryRefreshToken(); // Coalesce concurrent 401 retries onto a single refresh attempt.
if (!_refreshPromise) {
_refreshPromise = tryRefreshToken().finally(() => {
_refreshPromise = null;
});
}
const refreshed = await _refreshPromise;
if (!refreshed) { if (!refreshed) {
signOut(); signOut();
return null; return null;
@@ -899,7 +912,9 @@ function renderList() {
if (!item?.plain?.totp_uri) return; if (!item?.plain?.totp_uri) return;
const code = await getTotpCode(item.plain.totp_uri).catch(() => null); const code = await getTotpCode(item.plain.totp_uri).catch(() => null);
if (code) { if (code) {
navigator.clipboard.writeText(code); // Use _copyWithAutoClear so the 2FA code is wiped from the clipboard
// after 30 s, consistent with password copy behaviour.
_copyWithAutoClear(code);
btn.title = "Copied!"; btn.title = "Copied!";
btn.style.color = "#16a34a"; btn.style.color = "#16a34a";
setTimeout(() => { setTimeout(() => {
@@ -1163,7 +1178,7 @@ async function saveCredential(data) {
const res = await apiFetch("/api/vault", { const res = await apiFetch("/api/vault", {
method: "POST", method: "POST",
body: JSON.stringify({ body: JSON.stringify({
name: "password", name, // actual site name (plaintext fallback for legacy clients)
item_type: "password", item_type: "password",
folder_id, folder_id,
enc_data, enc_data,
@@ -1232,9 +1247,9 @@ function generatePassword(length, useLower, useUpper, useNumbers, useSymbols) {
if (useSymbols) if (useSymbols)
required.push(GEN_SETS.symbols[_cryptoRandInt(GEN_SETS.symbols.length)]); required.push(GEN_SETS.symbols[_cryptoRandInt(GEN_SETS.symbols.length)]);
const arr = new Uint32Array(length); // Use _cryptoRandInt() for each character to avoid modulo bias that arises
crypto.getRandomValues(arr); // when pool.length is not a power of 2.
const rest = Array.from(arr).map((n) => pool[n % pool.length]); const rest = Array.from({ length }, () => pool[_cryptoRandInt(pool.length)]);
// Splice required chars into random positions and trim to length. // Splice required chars into random positions and trim to length.
const combined = [...rest]; const combined = [...rest];
@@ -1462,7 +1477,7 @@ async function addItemToVault() {
const res = await apiFetch("/api/vault", { const res = await apiFetch("/api/vault", {
method: "POST", method: "POST",
body: JSON.stringify({ body: JSON.stringify({
name: "password", name, // actual site name (plaintext fallback for legacy clients)
item_type: "password", item_type: "password",
folder_id, folder_id,
enc_data, enc_data,
@@ -0,0 +1,63 @@
"""add recovery_challenges table
Revision ID: e5f6a7b8c9d0
Revises: d4e5f6a7b8c9
Create Date: 2026-05-17 00:00:00.000000
Adds the recovery_challenges table, which stores the challenge-response
state for account recovery (nonce + expected HMAC proof) in the database
rather than the Flask session cookie.
Why: Flask session cookies are per-worker in Gunicorn. A challenge written
by worker A is invisible to worker B, so the recovery flow would fail with
"No active recovery challenge" in any multi-worker deployment.
The table has a UNIQUE constraint on user_id (one active challenge per user)
and an index on expires_at so the APScheduler cleanup job can efficiently
delete expired rows without a full scan.
"""
from alembic import op
import sqlalchemy as sa
revision = 'e5f6a7b8c9d0'
down_revision = 'd4e5f6a7b8c9'
branch_labels = None
depends_on = None
def upgrade():
op.create_table(
'recovery_challenges',
sa.Column('id', sa.Integer(), nullable=False, autoincrement=True),
sa.Column('user_id', sa.Integer(), nullable=False),
sa.Column('nonce', sa.String(64), nullable=False),
sa.Column('expected_proof', sa.String(64), nullable=False),
sa.Column('expires_at', sa.DateTime(), nullable=False),
sa.ForeignKeyConstraint(
['user_id'], ['users.id'],
name='fk_recovery_challenges_user_id',
ondelete='CASCADE',
),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('user_id', name='uq_recovery_challenges_user_id'),
mysql_charset='utf8mb4',
)
op.create_index(
'ix_recovery_challenges_user_id',
'recovery_challenges',
['user_id'],
unique=True,
)
op.create_index(
'ix_recovery_challenges_expires_at',
'recovery_challenges',
['expires_at'],
unique=False,
)
def downgrade():
op.drop_index('ix_recovery_challenges_expires_at', table_name='recovery_challenges')
op.drop_index('ix_recovery_challenges_user_id', table_name='recovery_challenges')
op.drop_table('recovery_challenges')