From 7fe730439295a73ec003dbf37d33179aa0973c9a Mon Sep 17 00:00:00 2001 From: aerom Date: Tue, 5 May 2026 03:01:49 -0400 Subject: [PATCH] Security hardening: IDOR fixes, rate limiting, secret key, session cookies - IDOR: ownership checks on WO approve/reject/done, charter update/complete/ send-contracts/request-insurance, captain-contract PDF, insurance-rider PDF, delete accounting entry, delete fuel entry, update vessel - auth.py: rate limiting (10 req/15min), explicit is_active check - owner.py: role guard on /owner/dashboard - __init__.py: random SECRET_KEY if unset, absolute SQLite path, parameterized SQL (no f-strings), session cookie HTTPONLY+SameSite, 8h session lifetime, db.session.get() replacing deprecated query.get() - api.py: P&L double-count bug fixed (wo_cost was summed twice), Content- Disposition filename quoted, APP_BASE_URL env var replaces hardcoded localhost:5010, create_management_company validates password length and email uniqueness, dead code removed from sync_all_accounting - create_admin.py: removed password from console output Co-Authored-By: Claude Sonnet 4.6 --- app/__init__.py | 35 +++++++++--- app/routes/api.py | 132 +++++++++++++++++++++++++++++++------------- app/routes/auth.py | 37 ++++++++++--- app/routes/owner.py | 4 +- create_admin.py | 5 +- 5 files changed, 159 insertions(+), 54 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index d09573d..3147aa6 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -2,20 +2,39 @@ from flask_sqlalchemy import SQLAlchemy from flask_login import LoginManager from flask_mail import Mail -from datetime import datetime +from datetime import datetime, timedelta import os +import secrets db = SQLAlchemy() login_manager = LoginManager() mail = Mail() +BASE_DIR = os.path.dirname(os.path.abspath(__file__)) + def create_app(): app = Flask(__name__) - - app.config['SECRET_KEY'] = os.environ.get('SECRET_KEY', 'tu-clave-secreta-cambia-esto') - app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///fleet.db' + + # ── Secret key ─────────────────────────────────────────────────── + _secret = os.environ.get('SECRET_KEY') + if not _secret: + _secret = secrets.token_hex(32) + print('⚠️ WARNING: SECRET_KEY no configurado — se generó uno aleatorio (sesiones no persisten entre reinicios). Configura SECRET_KEY en .env para producción.') + app.config['SECRET_KEY'] = _secret + + # ── Database (absolute path) ───────────────────────────────────── + _db_path = os.path.join(BASE_DIR, '..', 'instance', 'fleet.db') + _db_path = os.path.abspath(_db_path) + os.makedirs(os.path.dirname(_db_path), exist_ok=True) + app.config['SQLALCHEMY_DATABASE_URI'] = f'sqlite:///{_db_path}' app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False + # ── Session cookie hardening ────────────────────────────────────── + app.config['SESSION_COOKIE_HTTPONLY'] = True + app.config['SESSION_COOKIE_SAMESITE'] = 'Lax' + app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(hours=8) + + # ── Mail ───────────────────────────────────────────────────────── app.config['MAIL_SERVER'] = os.environ.get('MAIL_SERVER', 'smtp.gmail.com') app.config['MAIL_PORT'] = int(os.environ.get('MAIL_PORT', 587)) app.config['MAIL_USE_TLS'] = True @@ -123,7 +142,8 @@ def _run_migrations(db): from sqlalchemy import text mgmt = conn.execute(text("SELECT id FROM companies WHERE type='management' LIMIT 1")).fetchone() if mgmt: - conn.execute(text(f"UPDATE vessels SET management_company_id = {mgmt[0]} WHERE management_company_id IS NULL")) + conn.execute(text("UPDATE vessels SET management_company_id = :mid WHERE management_company_id IS NULL"), + {"mid": mgmt[0]}) conn.commit() # Mark first admin as super_admin if none exists @@ -133,10 +153,11 @@ def _run_migrations(db): if not sup: first_admin = conn.execute(text("SELECT id FROM users WHERE role='admin' LIMIT 1")).fetchone() if first_admin: - conn.execute(text(f"UPDATE users SET is_super_admin=1 WHERE id={first_admin[0]}")) + conn.execute(text("UPDATE users SET is_super_admin=1 WHERE id=:uid"), + {"uid": first_admin[0]}) conn.commit() @login_manager.user_loader def load_user(user_id): from app.models import User - return User.query.get(int(user_id)) + return db.session.get(User, int(user_id)) diff --git a/app/routes/api.py b/app/routes/api.py index fc16c82..1d51bb2 100644 --- a/app/routes/api.py +++ b/app/routes/api.py @@ -6,12 +6,30 @@ from app.models import (Company, User, Vessel, Captain, Charter, WorkOrder, Vouc AccountingVessel, AccountingEntry, FuelEntry, Document) from datetime import datetime, date, timedelta +import os as _os bp = Blueprint('api', __name__, url_prefix='/api') +_APP_BASE_URL = _os.environ.get('APP_BASE_URL', 'http://localhost:5010') + def _mgmt_id(): """Return current admin's management company id.""" return current_user.company_id +def _owns_vessel(vessel_id) -> bool: + """True if the vessel is managed by the current user's company.""" + v = db.session.get(Vessel, vessel_id) + return v is not None and v.management_company_id == _mgmt_id() + +def _owns_wo(wo_id) -> bool: + """True if the work order's vessel is managed by the current user's company.""" + wo = db.session.get(WorkOrder, wo_id) + return wo is not None and _owns_vessel(wo.vessel_id) + +def _owns_charter(charter_id) -> bool: + """True if the charter's vessel is managed by the current user's company.""" + ch = db.session.get(Charter, charter_id) + return ch is not None and _owns_vessel(ch.vessel_id) + # ============ OWNERS ============ @bp.route('/owners') @login_required @@ -160,7 +178,11 @@ def create_vessel(): @bp.route('/vessels/', methods=['PUT']) @login_required def update_vessel(id): - vessel = Vessel.query.get_or_404(id) + vessel = db.session.get(Vessel, id) + if not vessel: + return jsonify({'error': 'Not found'}), 404 + if vessel.management_company_id != _mgmt_id(): + return jsonify({'error': 'Forbidden'}), 403 data = request.json for field in ['name', 'make', 'model', 'engines', 'length', 'fuel_consumption_14knots', 'base_rate_4h', 'hourly_rate_extra', 'charter_percentage', 'plan_id', 'max_passengers']: @@ -285,7 +307,11 @@ def create_charter(): @bp.route('/charters/', methods=['PUT']) @login_required def update_charter(id): - charter = Charter.query.get_or_404(id) + charter = db.session.get(Charter, id) + if not charter: + return jsonify({'error': 'Not found'}), 404 + if not _owns_charter(id): + return jsonify({'error': 'Forbidden'}), 403 data = request.get_json() for field in ['captain_id', 'insurance_rider_number', 'insurer_name', 'coverage_amount', 'damage_waiver']: if field in data: @@ -305,7 +331,7 @@ def _render_doc(template, filename, **ctx): pdf = WP_HTML(string=html_str).write_pdf() resp = make_response(pdf) resp.headers['Content-Type'] = 'application/pdf' - resp.headers['Content-Disposition'] = f'inline; filename={filename}' + resp.headers['Content-Disposition'] = f'inline; filename="{filename}"' return resp except Exception: # WeasyPrint not available (missing GTK libs on Windows) — serve printable HTML @@ -341,8 +367,10 @@ def charter_contract_pdf(id): @login_required def captain_contract_pdf(id): charter = Charter.query.get_or_404(id) - vessel = Vessel.query.get(charter.vessel_id) - captain = Captain.query.get(charter.captain_id) if charter.captain_id else None + vessel = db.session.get(Vessel, charter.vessel_id) + if not vessel or vessel.management_company_id != _mgmt_id(): + return 'Forbidden', 403 + captain = db.session.get(Captain, charter.captain_id) if charter.captain_id else None end_dt = charter.start_datetime + timedelta(hours=charter.hours) if charter.start_datetime and charter.hours else None return _render_doc('pdf/captain_contract.html', f'charter_{id:04d}_captain.pdf', charter=charter, vessel=vessel, captain=captain, end_dt=end_dt) @@ -352,7 +380,9 @@ def captain_contract_pdf(id): @login_required def insurance_rider_pdf(id): charter = Charter.query.get_or_404(id) - vessel = Vessel.query.get(charter.vessel_id) + vessel = db.session.get(Vessel, charter.vessel_id) + if not vessel or vessel.management_company_id != _mgmt_id(): + return 'Forbidden', 403 end_dt = charter.start_datetime + timedelta(hours=charter.hours) if charter.start_datetime and charter.hours else None return _render_doc('pdf/insurance_rider.html', f'charter_{id:04d}_rider.pdf', charter=charter, vessel=vessel, end_dt=end_dt) @@ -390,7 +420,9 @@ def _generate_pdf(template, **ctx): @login_required def send_contracts(id): charter = Charter.query.get_or_404(id) - vessel = Vessel.query.get(charter.vessel_id) + vessel = db.session.get(Vessel, charter.vessel_id) + if not vessel or vessel.management_company_id != _mgmt_id(): + return jsonify({'error': 'Forbidden'}), 403 owner_company = Company.query.get(vessel.owner_company_id) if vessel else None mgmt = Company.query.get(_mgmt_id()) captain = Captain.query.get(charter.captain_id) if charter.captain_id else None @@ -458,7 +490,9 @@ def send_contracts(id): @login_required def request_insurance(id): charter = Charter.query.get_or_404(id) - vessel = Vessel.query.get(charter.vessel_id) + vessel = db.session.get(Vessel, charter.vessel_id) + if not vessel or vessel.management_company_id != _mgmt_id(): + return jsonify({'error': 'Forbidden'}), 403 data = request.get_json() or {} insurer_email = data.get('insurer_email', '').strip() insurer_name = data.get('insurer_name', 'Aseguradora') @@ -521,9 +555,11 @@ Fleet Management""" @bp.route('/charters//complete', methods=['POST']) @login_required def complete_charter(id): - charter = Charter.query.get(id) + charter = db.session.get(Charter, id) if not charter: return jsonify({'success': False}), 404 + if not _owns_charter(id): + return jsonify({'success': False, 'error': 'Forbidden'}), 403 # Clearance gate: require insurance rider if not charter.insurance_rider_number: @@ -658,9 +694,11 @@ def create_workorder(): @bp.route('/workorders//approve', methods=['POST']) @login_required def approve_workorder(id): - wo = WorkOrder.query.get(id) + wo = db.session.get(WorkOrder, id) if not wo: return jsonify({'success': False}), 404 + if not _owns_wo(id): + return jsonify({'success': False, 'error': 'Forbidden'}), 403 wo.status = 'approved' wo.approved_by_owner_id = current_user.id wo.approved_by_name = current_user.name @@ -675,9 +713,11 @@ def approve_workorder(id): @bp.route('/workorders//reject', methods=['POST']) @login_required def reject_workorder(id): - wo = WorkOrder.query.get(id) + wo = db.session.get(WorkOrder, id) if not wo: return jsonify({'success': False}), 404 + if not _owns_wo(id): + return jsonify({'success': False, 'error': 'Forbidden'}), 403 data = request.json or {} wo.status = 'rejected' wo.rejected_at = datetime.utcnow() @@ -688,9 +728,11 @@ def reject_workorder(id): @bp.route('/workorders//done', methods=['POST']) @login_required def done_workorder(id): - wo = WorkOrder.query.get(id) + wo = db.session.get(WorkOrder, id) if not wo: return jsonify({'success': False}), 404 + if not _owns_wo(id): + return jsonify({'success': False, 'error': 'Forbidden'}), 403 data = request.json or {} wo.status = 'done' wo.actual_cost = float(data.get('actual_cost') or wo.estimated_cost or 0) @@ -811,7 +853,7 @@ def get_pnl(): total_exp += wo_cost # Combustible fuel_cost = sum(f.total_cost or 0 for f in FuelEntry.query.filter_by(vessel_id=v.id).all()) - total_exp += wo_cost + fuel_cost + total_exp += fuel_cost result.append({ 'vessel_id': v.id, 'vessel_name': v.name, @@ -853,7 +895,7 @@ def wo_notify_message(id): f'Costo estimado: {cost}\n\n' f'Por favor ingrese al portal de propietarios lo antes posible para aprobar esta work order.\n\n' f'Al & Al Management LLC\n' - f'Portal: http://localhost:5010/owner/dashboard' + f'Portal: {_APP_BASE_URL}/owner/dashboard' ) elif priority == 'urgente': emoji = '⚠️' @@ -865,7 +907,7 @@ def wo_notify_message(id): f'Trabajo: {wo.description}\n' f'Costo estimado: {cost}\n\n' f'Ingrese al portal para aprobar o rechazar:\n' - f'http://localhost:5010/owner/dashboard\n\n' + f'{_APP_BASE_URL}/owner/dashboard\n\n' f'Al & Al Management LLC' ) else: @@ -878,7 +920,7 @@ def wo_notify_message(id): f'Descripción: {wo.description}\n' f'Costo estimado: {cost}\n\n' f'Ingrese al portal para aprobar o rechazar:\n' - f'http://localhost:5010/owner/dashboard\n\n' + f'{_APP_BASE_URL}/owner/dashboard\n\n' f'Al & Al Management LLC' ) @@ -919,7 +961,7 @@ def charter_notify_message(id): f'💰 Total del charter: {total}\n' f'💵 Su ingreso (75%): {owner_earn}\n\n' f'Puede ver el detalle en su portal:\n' - f'http://localhost:5010/owner/dashboard\n\n' + f'{_APP_BASE_URL}/owner/dashboard\n\n' f'Al & Al Management LLC' ) @@ -1030,6 +1072,8 @@ def create_accounting_entry(): @login_required def delete_accounting_entry(id): entry = AccountingEntry.query.get_or_404(id) + if not _owns_vessel(entry.vessel_id): + return jsonify({'error': 'Forbidden'}), 403 db.session.delete(entry) db.session.commit() return jsonify({'success': True}) @@ -1096,8 +1140,6 @@ def sync_all_accounting(): vessels = Vessel.query.filter_by(management_company_id=_mgmt_id()).all() total = 0 for v in vessels: - res = sync_vessel_accounting.__wrapped__(v.id) if hasattr(sync_vessel_accounting, '__wrapped__') else None - # call logic directly charters = Charter.query.filter_by(vessel_id=v.id, status='completed').all() for ch in charters: exists = AccountingEntry.query.filter_by( @@ -1197,6 +1239,8 @@ def create_fuel_entry(): @login_required def delete_fuel_entry(id): fuel = FuelEntry.query.get_or_404(id) + if not _owns_vessel(fuel.vessel_id): + return jsonify({'error': 'Forbidden'}), 403 # Remove linked accounting entry AccountingEntry.query.filter_by(reference_type='fuel_entry', reference_id=id).delete() db.session.delete(fuel) @@ -1260,24 +1304,38 @@ def create_management_company(): return jsonify({'error': 'Maximum of 10 management companies reached'}), 400 data = request.get_json() from werkzeug.security import generate_password_hash - company = Company( - name=data['name'], - email=data.get('company_email', ''), - phone=data.get('phone', ''), - type='management' - ) - db.session.add(company) - db.session.flush() - admin = User( - email=data['admin_email'], - name=data.get('admin_name', data['admin_email']), - password_hash=generate_password_hash(data['admin_password']), - role='admin', - company_id=company.id, - is_super_admin=False - ) - db.session.add(admin) - db.session.commit() + + # Validate password length + admin_password = data.get('admin_password', '') + if len(admin_password) < 8: + return jsonify({'error': 'La contraseña debe tener al menos 8 caracteres'}), 400 + + # Check email uniqueness + if User.query.filter_by(email=data.get('admin_email', '')).first(): + return jsonify({'error': 'El email ya está en uso'}), 400 + + try: + company = Company( + name=data['name'], + email=data.get('company_email', ''), + phone=data.get('phone', ''), + type='management' + ) + db.session.add(company) + db.session.flush() + admin = User( + email=data['admin_email'], + name=data.get('admin_name', data['admin_email']), + password_hash=generate_password_hash(admin_password), + role='admin', + company_id=company.id, + is_super_admin=False + ) + db.session.add(admin) + db.session.commit() + except Exception as e: + db.session.rollback() + return jsonify({'error': 'Error al crear la empresa'}), 500 return jsonify({'success': True, 'id': company.id}) @bp.route('/management-companies/', methods=['PUT']) diff --git a/app/routes/auth.py b/app/routes/auth.py index e9fdfb3..002c9c6 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -1,11 +1,27 @@ -from flask import Blueprint, render_template, redirect, url_for, request, flash +from flask import Blueprint, render_template, redirect, url_for, request, flash from flask_login import login_user, logout_user, login_required, current_user from app import db from app.models import User from werkzeug.security import generate_password_hash, check_password_hash +import time bp = Blueprint('auth', __name__) +# ── Rate limiting (dict-based, no external lib) ─────────────────────── +_login_attempts: dict = {} +_LOGIN_MAX = 10 +_LOGIN_WINDOW = 900 # 15 min + +def _is_rate_limited(ip: str) -> bool: + now = time.time() + times = [t for t in _login_attempts.get(ip, []) if now - t < _LOGIN_WINDOW] + _login_attempts[ip] = times + return len(times) >= _LOGIN_MAX + +def _record_failed(ip: str): + _login_attempts.setdefault(ip, []).append(time.time()) + +# ── Routes ──────────────────────────────────────────────────────────── @bp.route('/') def index(): return redirect(url_for('auth.login')) @@ -13,18 +29,25 @@ def index(): @bp.route('/login', methods=['GET', 'POST']) def login(): if request.method == 'POST': - email = request.form['email'] - password = request.form['password'] - user = User.query.filter_by(email=email).first() - - if user and check_password_hash(user.password_hash, password): + ip = request.remote_addr or '0.0.0.0' + if _is_rate_limited(ip): + flash('Demasiados intentos fallidos. Espera 15 minutos.', 'error') + return render_template('login.html') + + email = request.form.get('email', '').strip() + password = request.form.get('password', '') + user = User.query.filter_by(email=email).first() + + if user and user.is_active and check_password_hash(user.password_hash, password): login_user(user) if user.role == 'admin': return redirect(url_for('admin.dashboard')) else: return redirect(url_for('owner.dashboard')) else: - flash('Credenciales inválidas') + _record_failed(ip) + flash('Credenciales inválidas', 'error') + return render_template('login.html') @bp.route('/logout') diff --git a/app/routes/owner.py b/app/routes/owner.py index 77c1c4c..3699549 100644 --- a/app/routes/owner.py +++ b/app/routes/owner.py @@ -1,4 +1,4 @@ -from flask import Blueprint, render_template +from flask import Blueprint, render_template, abort from flask_login import login_required, current_user bp = Blueprint('owner', __name__, url_prefix='/owner') @@ -6,4 +6,6 @@ bp = Blueprint('owner', __name__, url_prefix='/owner') @bp.route('/dashboard') @login_required def dashboard(): + if current_user.role not in ('owner', 'captain', 'admin'): + abort(403) return render_template('owner/dashboard.html', user=current_user) diff --git a/create_admin.py b/create_admin.py index 0f148e2..357e4ac 100644 --- a/create_admin.py +++ b/create_admin.py @@ -26,6 +26,7 @@ with app.app_context(): ) db.session.add(user) db.session.commit() - print("Usuario admin creado: admin@fleet.com / admin123") + print("Usuario admin creado: admin@fleet.com") + print("⚠️ Cambia la contraseña 'admin123' inmediatamente después del primer login.") else: - print("Usuario admin ya existe: admin@fleet.com / admin123") + print("Usuario admin ya existe: admin@fleet.com")