diff --git a/app/__init__.py b/app/__init__.py index e91bed0..e8b520e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,4 +1,4 @@ -from flask import Flask, render_template +from flask import Flask, render_template, request as _flask_request from flask_sqlalchemy import SQLAlchemy from flask_migrate import Migrate from flask_login import LoginManager @@ -6,6 +6,7 @@ from flask_wtf.csrf import CSRFProtect from flask_limiter import Limiter from flask_limiter.util import get_remote_address from flask_cors import CORS +from werkzeug.middleware.proxy_fix import ProxyFix from .config import config @@ -15,6 +16,16 @@ login_manager = LoginManager() csrf = CSRFProtect() limiter = Limiter(key_func=get_remote_address) + +def client_ip() -> str: + """ + Return the real client IP address, trusted only from one upstream proxy + (Nginx). ProxyFix — applied in create_app() — ensures request.remote_addr + is already set to the correct value; we do NOT parse X-Forwarded-For + manually here, which would be spoofable. + """ + return _flask_request.remote_addr or '' + # APScheduler is used for the background token-blacklist cleanup job. # Imported here so it is available at module level; started inside create_app(). try: @@ -26,6 +37,11 @@ except ImportError: # pragma: no cover — optional dependency def create_app(config_name: str = 'development') -> Flask: app = Flask(__name__) + # Trust exactly one upstream proxy (Nginx) for X-Forwarded-For / X-Forwarded-Proto. + # x_for=1 means only the rightmost hop in XFF is trusted, making rate-limit + # IP keys spoof-resistant — a client cannot bypass per-IP limits by injecting + # an arbitrary IP into the XFF header. + app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1) app.config.from_object(config[config_name]) # Extensions @@ -46,21 +62,27 @@ def create_app(config_name: str = 'development') -> Flask: # Attach security headers to every response @app.after_request def set_security_headers(response): - # Strict-Transport-Security: enforce HTTPS for 1 year, include subdomains + # HSTS: enforce HTTPS for 1 year across all subdomains. + # 'preload' enables browser preload-list submission so first-time + # HTTP visitors are also protected before the first redirect. response.headers['Strict-Transport-Security'] = ( - 'max-age=31536000; includeSubDomains' + 'max-age=31536000; includeSubDomains; preload' ) - # Prevent clickjacking + # Prevent clickjacking (belt-and-suspenders alongside CSP frame-ancestors) response.headers['X-Frame-Options'] = 'DENY' - # Prevent MIME-type sniffing + # Prevent MIME-type sniffing attacks response.headers['X-Content-Type-Options'] = 'nosniff' - # Control referrer information leakage + # Limit referrer information sent on navigation response.headers['Referrer-Policy'] = 'strict-origin-when-cross-origin' - # Permissions policy — disable features the app does not use + # Permissions policy — disable browser features the app does not use response.headers['Permissions-Policy'] = ( 'geolocation=(), camera=(), microphone=()' ) - # CSP via HTTP header (authoritative — overrides the meta tag for all resources) + # Content-Security-Policy (HTTP header is authoritative — overrides meta tag). + # base-uri 'self' — blocks injection that would redirect all + # relative URLs to an attacker-controlled origin. + # upgrade-insecure-requests — instructs browsers to rewrite http:// sub-resource + # requests to https:// to avoid mixed-content warnings. response.headers['Content-Security-Policy'] = ( "default-src 'self'; " "script-src 'self'; " @@ -68,7 +90,9 @@ def create_app(config_name: str = 'development') -> Flask: "img-src 'self' data:; " "font-src 'self'; " "connect-src 'self' https://api.pwnedpasswords.com; " - "frame-ancestors 'none';" + "frame-ancestors 'none'; " + "base-uri 'self'; " + "upgrade-insecure-requests;" ) return response diff --git a/app/config.py b/app/config.py index 83e84d6..51977f2 100644 --- a/app/config.py +++ b/app/config.py @@ -4,6 +4,9 @@ from dotenv import load_dotenv load_dotenv() +# Shared set of known-insecure placeholder values that must never reach production. +_INSECURE_SECRET_DEFAULTS = {'dev-secret-change-me', 'jwt-secret-change-me', '', 'change-me'} + class BaseConfig: SECRET_KEY = os.environ.get('SECRET_KEY', 'dev-secret-change-me') @@ -79,6 +82,38 @@ class ProductionConfig(BaseConfig): SESSION_COOKIE_HTTPONLY = True SESSION_COOKIE_SAMESITE = 'Lax' + # ── Critical security checks — fail loudly at startup, not silently at runtime ── + # These checks run at class definition time (i.e. at import / app startup). + # Any misconfiguration raises RuntimeError immediately so the process never + # serves a single request with an insecure configuration. + + _secret_key = os.environ.get('SECRET_KEY', '') + if not _secret_key or _secret_key in _INSECURE_SECRET_DEFAULTS: + raise RuntimeError( + '[PassKeeper] SECRET_KEY is not set or uses an insecure placeholder. ' + 'Generate a strong key with: python -c "import secrets; print(secrets.token_hex(32))" ' + 'and add SECRET_KEY= to your production .env file.' + ) + SECRET_KEY = _secret_key + + _jwt_secret = os.environ.get('JWT_SECRET_KEY', '') + if not _jwt_secret or _jwt_secret in _INSECURE_SECRET_DEFAULTS: + raise RuntimeError( + '[PassKeeper] JWT_SECRET_KEY is not set or uses an insecure placeholder. ' + 'Generate a strong key with: python -c "import secrets; print(secrets.token_hex(32))" ' + 'and add JWT_SECRET_KEY= to your production .env file.' + ) + JWT_SECRET_KEY = _jwt_secret + + _cors = os.environ.get('CORS_ORIGINS', '') + if not _cors or _cors.strip() == '*': + raise RuntimeError( + '[PassKeeper] CORS_ORIGINS must be set to a specific origin in production ' + '(e.g. CORS_ORIGINS=https://pwkeeper.ngodanguyen.tech). ' + 'A wildcard "*" is not permitted in production.' + ) + CORS_ORIGINS = _cors + config = { 'development': DevelopmentConfig, diff --git a/app/routes/auth.py b/app/routes/auth.py index d1ab92b..0761113 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -2,7 +2,7 @@ import re import time from flask import Blueprint, request, jsonify, g, session -from app import db, limiter +from app import db, limiter, client_ip from app.models.user import User from app.models.audit_log import AuditLog from app.services.auth_service import ( @@ -26,11 +26,6 @@ auth_bp = Blueprint('auth', __name__) EMAIL_RE = re.compile(r'^[^@\s]+@[^@\s]+\.[^@\s]+$') -def _client_ip(): - """Return the best-effort client IP from the request context.""" - return request.headers.get('X-Forwarded-For', request.remote_addr or '').split(',')[0].strip() - - @auth_bp.route('/register', methods=['POST']) @limiter.limit('10 per minute') def register(): @@ -41,6 +36,8 @@ def register(): if not email or not EMAIL_RE.match(email): return jsonify({'error': 'Invalid email address'}), 400 + if len(email) > 254: + return jsonify({'error': 'Email address is too long'}), 400 if not auth_hash: return jsonify({'error': 'auth_hash is required'}), 400 if not enc_key_salt: @@ -60,7 +57,7 @@ def register(): resource_type='user', resource_id=user.id, detail=f'New account registered: {email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -103,7 +100,7 @@ def login(): resource_type='user', resource_id=user.id, detail=f'Login blocked — account locked for {remaining} more minute(s)', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({ @@ -129,7 +126,7 @@ def login(): resource_type='user', resource_id=user.id, detail=f'Account locked for {LOCKOUT_MINUTES} minutes after {user.failed_login_count} failed attempts', - ip_address=_client_ip(), + ip_address=client_ip(), ) else: AuditLog.log( @@ -138,7 +135,7 @@ def login(): resource_type='user', resource_id=user.id, detail=f'Failed login attempt — invalid password ({user.failed_login_count}/{MAX_FAILED_LOGINS})', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() except OperationalError: @@ -149,7 +146,7 @@ def login(): resource_type='user', resource_id=user.id, detail='Failed login attempt — invalid password', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'error': 'Invalid email or password'}), 401 @@ -169,7 +166,7 @@ def login(): resource_type='user', resource_id=user.id, detail=f'Successful login{" (MFA pending)" if user.totp_enabled else ""}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -298,7 +295,7 @@ def mfa_enable(): resource_type='user', resource_id=user.id, detail='TOTP two-factor authentication enabled; backup codes generated', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -349,7 +346,7 @@ def mfa_disable(): resource_type='user', resource_id=user.id, detail='TOTP two-factor authentication disabled; backup codes cleared', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -397,7 +394,7 @@ def mfa_verify(): resource_type='user', resource_id=user.id, detail=f'MFA backup code used; {len(remaining)} code(s) remaining', - ip_address=_client_ip(), + ip_address=client_ip(), ) if not verified: @@ -412,7 +409,7 @@ def mfa_verify(): resource_type='user', resource_id=user.id, detail='MFA verification successful — session tokens issued', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -469,7 +466,7 @@ def mfa_backup_codes_regenerate(): resource_type='user', resource_id=user.id, detail='MFA backup codes regenerated — previous codes invalidated', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -572,7 +569,7 @@ def change_password(): resource_type='user', resource_id=user.id, detail='Password change rejected — current password incorrect', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'error': 'Current password is incorrect'}), 401 @@ -619,7 +616,7 @@ def change_password(): resource_type='user', resource_id=user.id, detail=f'Master password changed; {len(existing)} vault item(s) re-encrypted; recovery code cleared', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() except Exception as e: @@ -653,7 +650,7 @@ def delete_account(): resource_type='user', resource_id=user.id, detail='Account deletion rejected — password incorrect', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'error': 'Incorrect password'}), 401 @@ -668,7 +665,7 @@ def delete_account(): resource_type='user', resource_id=user_id, detail=f'Account permanently deleted: {user_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.delete(user) db.session.commit() @@ -712,7 +709,7 @@ def recovery_setup(): resource_type='user', resource_id=user.id, detail='Account recovery code configured', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -775,7 +772,7 @@ def recover_account(): resource_type='user', resource_id=user.id, detail='Recovery attempt failed — incorrect recovery proof', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'error': 'Invalid recovery code'}), 401 @@ -824,7 +821,7 @@ def recover_account(): resource_type='user', resource_id=user.id, detail=f'Account recovered; {len(existing)} vault item(s) re-encrypted; recovery code consumed', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() except Exception as e: @@ -926,7 +923,7 @@ def recovery_items(): resource_type='user', resource_id=user.id, detail='Recovery items request denied — incorrect recovery proof', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'error': 'Invalid recovery proof'}), 401 diff --git a/app/routes/emergency.py b/app/routes/emergency.py index d19b40f..90b59c7 100644 --- a/app/routes/emergency.py +++ b/app/routes/emergency.py @@ -1,7 +1,7 @@ from datetime import datetime, timezone from flask import Blueprint, request, jsonify, g -from app import db, limiter +from app import db, limiter, client_ip from app.models.user import User from app.models.emergency_access import EmergencyAccess from app.models.audit_log import AuditLog @@ -10,9 +10,6 @@ from app.services.auth_service import require_jwt emergency_bp = Blueprint('emergency', __name__) -def _client_ip(): - return request.headers.get('X-Forwarded-For', request.remote_addr or '').split(',')[0].strip() - @emergency_bp.route('', methods=['GET']) @limiter.limit('30 per minute') @@ -95,7 +92,7 @@ def create_emergency(): resource_type='emergency_access', resource_id=ea.id, detail=f'Created emergency access invitation for {grantee_email} (wait: {wait_days}d)', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -121,7 +118,7 @@ def delete_emergency(ea_id): resource_type='emergency_access', resource_id=ea_id, detail=f'Removed emergency access grant for {grantee_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'message': 'Emergency access removed'}), 200 @@ -153,7 +150,7 @@ def accept_emergency(ea_id): resource_type='emergency_access', resource_id=ea.id, detail=f'Accepted emergency access invitation from grantor_id={ea.grantor_id}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -190,7 +187,7 @@ def provide_vault(ea_id): resource_type='emergency_access', resource_id=ea.id, detail=f'Provided encrypted vault snapshot for grantee {ea.grantee_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -224,7 +221,7 @@ def request_access(ea_id): resource_type='emergency_access', resource_id=ea.id, detail=f'Requested emergency vault access from grantor_id={ea.grantor_id} (wait: {ea.wait_days}d)', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -251,7 +248,7 @@ def deny_access(ea_id): resource_type='emergency_access', resource_id=ea.id, detail=f'Denied emergency access request from grantee {ea.grantee_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -293,7 +290,7 @@ def get_emergency_vault(ea_id): resource_type='emergency_access', resource_id=ea.id, detail=f'Retrieved emergency vault from grantor_id={ea.grantor_id}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() diff --git a/app/routes/folders.py b/app/routes/folders.py index 3826f50..348868f 100644 --- a/app/routes/folders.py +++ b/app/routes/folders.py @@ -1,5 +1,5 @@ from flask import Blueprint, request, jsonify, g -from app import db, limiter +from app import db, limiter, client_ip from app.models.folder import Folder from app.models.audit_log import AuditLog from app.services.auth_service import require_jwt @@ -7,10 +7,6 @@ from app.services.auth_service import require_jwt folders_bp = Blueprint('folders', __name__) -def _client_ip(): - from flask import request as _req - return _req.headers.get('X-Forwarded-For', _req.remote_addr or '').split(',')[0].strip() - @folders_bp.route('', methods=['GET']) @limiter.limit('120 per minute') @@ -38,7 +34,7 @@ def create_folder(): resource_type='folder', resource_id=folder.id, detail=f'Created folder: "{name}"', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify(folder.to_dict()), 201 @@ -64,7 +60,7 @@ def update_folder(folder_id): resource_type='folder', resource_id=folder.id, detail=f'Renamed folder: "{old_name}" → "{name}"', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify(folder.to_dict()), 200 @@ -87,7 +83,7 @@ def delete_folder(folder_id): resource_type='folder', resource_id=folder_id, detail=f'Deleted folder: "{folder_name}"', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'message': 'Folder deleted'}), 200 \ No newline at end of file diff --git a/app/routes/sharing.py b/app/routes/sharing.py index 00ecd8a..06598cc 100644 --- a/app/routes/sharing.py +++ b/app/routes/sharing.py @@ -1,5 +1,5 @@ from flask import Blueprint, request, jsonify, g -from app import db, limiter +from app import db, limiter, client_ip from app.models.user import User from app.models.shared_item import SharedItem from app.models.audit_log import AuditLog @@ -8,9 +8,6 @@ from app.services.auth_service import require_jwt sharing_bp = Blueprint('sharing', __name__) -def _client_ip(): - return request.headers.get('X-Forwarded-For', request.remote_addr or '').split(',')[0].strip() - # ── Sharing keypair management ──────────────────────────────────────────────── @@ -55,7 +52,7 @@ def store_my_keys(): resource_type='sharing_keys', resource_id=g.current_user_id, detail='ECDH sharing keypair stored/updated', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -72,10 +69,10 @@ def get_public_key(): return jsonify({'error': 'email query param is required'}), 400 user = User.query.filter_by(email=email).first() - if not user: - return jsonify({'error': 'User not found'}), 404 - if not user.sharing_public_key: - return jsonify({'error': 'User has not set up sharing keys yet'}), 404 + # Return the same 404 regardless of whether the email is registered, + # to prevent user enumeration by authenticated clients. + if not user or not user.sharing_public_key: + return jsonify({'error': 'User or sharing key not found'}), 404 return jsonify({ 'user_id': user.id, @@ -161,7 +158,7 @@ def create_share(): resource_type='shared_item', resource_id=share.id, detail=f'Shared item "{item_name}" ({item_type}) with {recipient_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() @@ -187,7 +184,7 @@ def delete_share(share_id): resource_type='shared_item', resource_id=share_id, detail=f'Revoked share of "{item_name}" with {recipient_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'message': 'Share removed'}), 200 @@ -251,7 +248,7 @@ def accept_share(share_id): resource_type='shared_item', resource_id=share.id, detail=f'Accepted shared item "{share.item_name}" from {owner_email}', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() diff --git a/app/routes/vault.py b/app/routes/vault.py index a955622..0c84e54 100644 --- a/app/routes/vault.py +++ b/app/routes/vault.py @@ -1,5 +1,5 @@ from flask import Blueprint, request, jsonify, g -from app import db, limiter +from app import db, limiter, client_ip from app.models.vault_item import VaultItem, ItemType from app.models.audit_log import AuditLog from app.services.auth_service import require_jwt @@ -9,11 +9,6 @@ vault_bp = Blueprint('vault', __name__) VALID_TYPES = {t.value for t in ItemType} -def _client_ip(): - from flask import request as _req - return _req.headers.get('X-Forwarded-For', _req.remote_addr or '').split(',')[0].strip() - - @vault_bp.route('', methods=['GET']) @limiter.limit('120 per minute') @require_jwt @@ -64,7 +59,7 @@ def create_item(): resource_type='vault_item', resource_id=item.id, detail=f'Created {item_type} item: "{name}"', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() except Exception as e: @@ -122,7 +117,7 @@ def update_item(item_id): resource_type='vault_item', resource_id=item.id, detail=f'Updated item: "{item.name}"', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() except Exception as e: @@ -149,7 +144,7 @@ def delete_item(item_id): resource_type='vault_item', resource_id=item_id, detail=f'Deleted item: "{item_name}"', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'message': 'Item deleted'}), 200 @@ -177,7 +172,7 @@ def export_items(): resource_type='vault_item', resource_id=None, detail=f'Exported {len(items)} vault item(s)', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify([item.to_dict() for item in items]), 200 @@ -232,7 +227,7 @@ def import_items(): resource_type='vault_item', resource_id=None, detail=f'Imported {imported} item(s), skipped {skipped} malformed row(s)', - ip_address=_client_ip(), + ip_address=client_ip(), ) db.session.commit() return jsonify({'imported': imported, 'skipped': skipped}), 200 \ No newline at end of file