diff --git a/Claude.md b/Claude.md index 9750026..6456df6 100644 --- a/Claude.md +++ b/Claude.md @@ -92,7 +92,8 @@ QR_Code_Management/ │ ├── __init__.py │ ├── helpers.py # Role decorators, QR generation, role/permission helpers │ ├── geocoding.py # Haversine distance, Google Maps client, reverse geocode -│ └── template_helpers.py # Context processors: get_employee_name, format_hours, etc. +│ ├── template_helpers.py # Context processors: get_employee_name, format_hours, etc. +│ └── excel_safety.py # Formula-injection guard for every Excel/CSV export (Set 23) │ ├── static/ │ ├── css/ @@ -121,6 +122,7 @@ QR_Code_Management/ ├── migration_photo_verification_toggle.py # Adds photo_verification_enabled to qr_codes ├── migration_PM_permissions.py # One-time: user_project/location_permissions tables ├── migration_dynamic_qr_locations.py # One-time: qr_type column + qr_code_locations table + ├── migration_attendance_indexes.py # attendance_data secondary indexes (online DDL, re-runnable) └── optimize_time_attendance_db.py # Standalone DB index optimization script ``` @@ -193,6 +195,21 @@ STAFF_LEVEL_ROLES = ['staff', 'payroll', 'project_manager', 'accounting'] **`/register` is restricted to `@admin_required`** — public self-registration is disabled. +**Server-side role enforcement (Sept 2026, Set 23)** — the sidebar in `base_authenticated.html` +is the reference for who may use what; routes enforce the same rules: +- `roles_required(*roles)` decorator and `restrict_blueprint_to_roles(bp, roles)` (a blueprint + `before_request`) in `utils/helpers.py`. Anonymous → login redirect; wrong role → flash + + dashboard redirect, or JSON 401/403 for `/api/`, `X-Requested-With` or JSON callers. +- `PAYROLL_AREA_ROLES = ('admin','payroll','accounting')` → whole blueprints: `time_attendance`, + `legacy_attendance`, `employees`, `statistics`. +- `QR_MANAGEMENT_ROLES = ('admin','staff','payroll','accounting')` → QR create / edit / bulk import + (not project managers). QR toggle / activate / deactivate → `admin` only (the dashboard shows + those buttons to admins only). +- User management state changes (delete, reactivate, promote, demote, activate, deactivate, + permanently-delete) are **POST-only** — never re-add GET: CSRF validation only runs on POST. +- New routes in a gated blueprint are covered automatically; new routes elsewhere need an + explicit decorator. + --- ## 5. Application Factory & Initialization Order @@ -227,7 +244,10 @@ STAFF_LEVEL_ROLES = ['staff', 'payroll', 'project_manager', 'accounting'] **Critical notes:** - `create_tables()` and `update_existing_qr_codes()` run inside `with app.app_context()` inside `create_app()` — they execute under gunicorn, not only under `__main__`. - `PerformanceMonitor` is imported lazily inside `__main__` only — gunicorn workers never load it. -- `update_existing_qr_codes()` prefers `QR_BASE_URL` from `.env`. Falls back to `FLASK_HOST`/`FLASK_PORT`. Never uses `request.url_root`. +- `update_existing_qr_codes()` builds startup QR images from `QR_BASE_URL` only. When it is not set, + missing images are **not** generated at startup (the old `FLASK_HOST`/`FLASK_PORT` fallback wrote + `http://localhost:5000/...` into QR codes). Note: the QR routes in `routes/qr_codes.py` build URLs + from `request.url_root`, not `QR_BASE_URL` — there is no `_get_qr_base_url()` helper despite older notes. --- @@ -503,7 +523,14 @@ Exception: `/qr//checkin` is CSRF-exempt (public, unauthenticated). - **CSRF-exempt:** `auth.login`, `auth.register`, `qr_codes.qr_checkin`, `static` ### Rate Limiting -- 5 failed login attempts within 15 minutes → IP blocked 15 minutes (in-memory per worker) +- Failed logins are counted over 15 minutes per **IP + username** (blocked after 5) and per + **username from any IP** (blocked after 20) — `SecurityManager.is_auth_rate_limited(username)`. + Not a plain per-IP counter, so users behind one shared IP cannot lock each other out. + In-memory per worker. The check that runs is in `auth.login`; the middleware's + `request.endpoint in ['login', ...]` check never matches (endpoint is `auth.login`) +- Client IP = `request.remote_addr` after `ProxyFix(x_for=TRUSTED_PROXY_COUNT)` in `create_app()`. + **Never read the first `X-Forwarded-For` entry** — the client controls it (that bypassed the limiter). + Behind Cloudflare + Nginx set `TRUSTED_PROXY_COUNT=2` - `SecurityManager.create_secure_session()` called on login success (clears failed-attempt counter) ### Session Security @@ -870,7 +897,11 @@ PHOTO_VERIFICATION_DISTANCE_THRESHOLD # Miles (default 0.3) VERIFICATION_PHOTO_MAX_SIZE # Bytes (default 5MB) UPLOAD_FOLDER # Temp path (default /tmp) DEFAULT_ADMIN_PASSWORD # CHANGE IN PRODUCTION -QR_BASE_URL # Public-facing domain for QR links — required behind reverse proxy +QR_BASE_URL # Public-facing domain — used for QR images generated at startup (none generated if unset) +TRUSTED_PROXY_COUNT # Reverse proxies in front of the app (default 1 = Nginx; 2 with Cloudflare; 0 = none) +MAX_UPLOAD_SIZE_MB # Whole-request cap, default 50 (Nginx client_max_body_size must be >= this) + # MAX_FORM_MEMORY_SIZE = VERIFICATION_PHOTO_MAX_SIZE + 1 MB (Flask 3.1 default 500 KB + # would 413 the base64 photo field) — set in config.py, not .env THEME_NAME # Activates static/css/theme-{THEME_NAME}.css SYNC_INTERVAL_MINUTES SQLALCHEMY_ENGINE_OPTIONS_POOL_SIZE / POOL_TIMEOUT / POOL_RECYCLE / MAX_OVERFLOW @@ -936,6 +967,7 @@ def run(): | `migration_photo_verification_toggle.py` | `qr_codes.photo_verification_enabled` TINYINT(1) DEFAULT 1 | | `migration_dynamic_qr_locations.py` | `qr_type` column + `qr_code_locations` table | | `migration_PM_permissions.py` | `user_project_permissions` + `user_location_permissions` tables | +| `migration_attendance_indexes.py` | `attendance_data` indexes: `idx_ad_date_time`, `idx_ad_emp_date_time`, `idx_ad_qr_emp_date`, `idx_ad_location_name`, `idx_ad_verif_status` — skips names/leading columns that already exist; online DDL with plain ALTER fallback (Set 23) | --- @@ -1271,6 +1303,22 @@ it (the workers share no pub/sub). | `static/js/qr_destination.js` | `disableFormImmediately()` replaced the Employee ID placeholder with "Checking location services..." and nothing restored it after location services were confirmed. The override is removed — the submit button already shows the checking state | | `templates/qr_destination.html` | Placeholder is now "Enter your employee ID / Ingrese su ID de empleado" | +### Set 23 — Review Step 1: Security, Limits, Indexes (Sept 15, 2026) +| File | Fix | +|---|---| +| `routes/users.py`, `templates/users.html`, `static/js/users.js` | 7 user-management actions POST-only (were GET+POST, so a forged link could promote/delete — CSRF only checks POST). Promote/demote links → CSRF forms with a `data-confirm` listener. `users.js` (not loaded by any template) switched to POST | +| `utils/helpers.py`, `routes/time_attendance.py`, `employees.py`, `legacy_attendance.py`, `statistics.py`, `qr_codes.py` | `roles_required` / `restrict_blueprint_to_roles`; role rules in §4. Previously any logged-in user (incl. project managers) could import/export time attendance, delete employees, create/deactivate QR codes | +| `advanced_security_middleware.py`, `routes/auth.py`, `app.py`, `config.py` | Login limiter: real IP via `ProxyFix`, counters per IP+username and per username (§10). The first `X-Forwarded-For` entry was trusted, so rotating it gave unlimited attempts | +| `utils/excel_safety.py` + `routes/attendance_export.py`, `routes/time_attendance_export.py`, `legacy_attendance_service.py`, `routes/statistics.py` | Formula injection: `excel_hyperlink()` escapes `"` in HYPERLINK arguments; `neutralize_unexpected_formulas(wb)` before every export `wb.save` stores any formula not matching the app's own shapes (Google Maps HYPERLINK, SUM, SUMIF) as text; `csv_safe()` in the statistics CSV. **New formula types in exports must be added to `_ALLOWED_FORMULA_PATTERNS`, or they are written as text** | +| `routes/qr_codes.py` | Check-in refuses an employee ID whose base is not 1–4 ASCII digits (was: only counted digits, so `=HYPERLINK(...)` passed into attendance_data) | +| `location_logging.py` | `/api/location-debug-info` admin-only; no longer echoes request headers (incl. the session Cookie) | +| `app.py`, `config.py` | Startup QR images use `QR_BASE_URL` or are skipped (no more `localhost` QR codes); `MAX_FORM_MEMORY_SIZE` / `MAX_CONTENT_LENGTH`; `@app.errorhandler(413)` (bilingual JSON for `/qr/` + `/api/`, flash + same-host redirect otherwise) | +| `static/js/qr_destination.js` | `updateSubmitButton()` looked for `#submitCheckin`, but the button is `#submitButton`, so the post-clone submit path never showed "Processing" — now finds either, bilingual label, restores via `renderSubmitButton()` | +| `routes/attendance.py` | Report query selects `NULL AS verification_photo` (the page never used the base64 photos it loaded for up to 1,000 rows) | +| `tools/migration_attendance_indexes.py` | New migration — see §17 | +| — | Verified offline (67 checks): formula guard incl. openpyxl save/reload, role gates per role and for JSON callers, limiter behind ProxyFix (spoofed XFF, shared IP, per-username cap, reset on login), debug endpoint, check-in ID rule, config limits, POST-only routes, migration against a fake cursor (create / covered / re-run / fallback / missing column). Not run against MySQL, Nginx or a browser | +| — | Known, not changed: `templates/confirm_delete_qr.html` links (GET) to the POST-only `deactivate_qr_code`, so that button returns 405 | + --- ## 21. Infrastructure & Deployment diff --git a/advanced_security_middleware.py b/advanced_security_middleware.py index 6f15ee5..5db385c 100644 --- a/advanced_security_middleware.py +++ b/advanced_security_middleware.py @@ -37,7 +37,8 @@ class SecurityManager: self.session_tokens = {} # Security configuration - self.max_failed_attempts = 5 + self.max_failed_attempts = 5 # per IP + username, 15 minutes + self.max_failed_attempts_per_username = 20 # per username from any IP, 15 minutes self.lockout_duration = 900 # 15 minutes self.session_timeout = 3600 # 1 hour @@ -117,12 +118,13 @@ class SecurityManager: def get_client_ip(self): """Get real client IP address""" - # Check for forwarded headers (in case behind proxy/CDN) - forwarded_ips = request.headers.getlist('X-Forwarded-For') - if forwarded_ips: - return forwarded_ips[0].split(',')[0].strip() + # request.remote_addr is the real client address: app.py wraps the app in + # ProxyFix(x_for=TRUSTED_PROXY_COUNT), which takes it from the entry our + # own proxy (Nginx) appended to X-Forwarded-For. The FIRST entry of that + # header — used here before — is whatever the client chose to send, so + # changing it on every request bypassed the login rate limit. - return request.headers.get('X-Real-IP') or request.remote_addr + return request.remote_addr or 'unknown' def is_suspicious_request(self): """Detect suspicious request patterns""" @@ -235,30 +237,52 @@ class SecurityManager: return False - def is_auth_rate_limited(self): - """Check if authentication endpoint is rate limited""" + def _login_attempt_keys(self, username=None): + """Failed-login counter keys: this IP + username, and the username alone.""" client_ip = self.get_client_ip() + name = (username or '').strip().lower() + if not name: + return [f"ip:{client_ip}"] + return [f"ip:{client_ip}|user:{name}", f"user:{name}"] + + def is_auth_rate_limited(self, username=None): + """ + Should this login attempt be blocked? + + Two counters over lockout_duration (15 minutes): + - this IP + username: blocked after max_failed_attempts (5) + - this username from ANY IP: blocked after max_failed_attempts_per_username + (20), so rotating IP addresses cannot brute-force one account + Deliberately not a plain per-IP counter: users behind one shared address + (office NAT, a proxy that hides client IPs) must not lock each other out. + Counters are in memory per gunicorn worker. + """ current_time = time.time() - - # Clean old attempts - self.failed_attempts[client_ip] = deque([ - attempt for attempt in self.failed_attempts[client_ip] - if current_time - attempt < 900 # Keep attempts from last 15 minutes - ], maxlen=10) - - return len(self.failed_attempts[client_ip]) >= self.max_failed_attempts - + for key in self._login_attempt_keys(username): + self.failed_attempts[key] = deque([ + attempt for attempt in self.failed_attempts.get(key, ()) + if current_time - attempt < self.lockout_duration + ], maxlen=50) + limit = (self.max_failed_attempts_per_username if key.startswith('user:') + else self.max_failed_attempts) + if len(self.failed_attempts[key]) >= limit: + return True + return False + def record_failed_attempt(self, identifier): - """Record a failed authentication attempt""" + """Record a failed authentication attempt (identifier = the submitted username)""" client_ip = self.get_client_ip() current_time = time.time() - - self.failed_attempts[client_ip].append(current_time) - + keys = self._login_attempt_keys(identifier) + for key in keys: + if key not in self.failed_attempts: + self.failed_attempts[key] = deque(maxlen=50) + self.failed_attempts[key].append(current_time) + self.log_security_event('authentication_failure', { 'ip': client_ip, 'identifier': identifier, - 'attempts': len(self.failed_attempts[client_ip]) + 'attempts': len(self.failed_attempts[keys[0]]) }) def create_secure_session(self, user_id): @@ -278,10 +302,10 @@ class SecurityManager: session['security_token'] = session_token session['login_time'] = datetime.utcnow().isoformat() - # Clear any failed attempts for this IP + # Clear the failed-login counters for this IP + username and the username client_ip = self.get_client_ip() - if client_ip in self.failed_attempts: - del self.failed_attempts[client_ip] + for key in self._login_attempt_keys(session.get('username')) + [f"ip:{client_ip}"]: + self.failed_attempts.pop(key, None) self.log_security_event('secure_session_created', { 'user_id': user_id, diff --git a/app.py b/app.py index 6cc809f..a1e861d 100644 --- a/app.py +++ b/app.py @@ -48,6 +48,14 @@ def create_app() -> Flask: cfg = get_config() app.config.from_object(cfg) + # Real client IP behind Nginx: ProxyFix takes it from the X-Forwarded-For + # entry appended by our own proxy (TRUSTED_PROXY_COUNT hops), never from the + # first, client-supplied entry. The login rate limiter depends on it. + _trusted_proxies = app.config.get('TRUSTED_PROXY_COUNT', 1) + if _trusted_proxies > 0: + from werkzeug.middleware.proxy_fix import ProxyFix + app.wsgi_app = ProxyFix(app.wsgi_app, x_for=_trusted_proxies) + # Guard against deployment with the insecure default SECRET_KEY import sys if not app.debug and app.config.get('SECRET_KEY') == 'change-me-in-production': @@ -324,6 +332,20 @@ def create_app() -> Flask: """Handle internal server errors with user-friendly page""" return render_template('errors/500.html'), 500 + @app.errorhandler(413) + def request_too_large(error): + """Request over MAX_CONTENT_LENGTH, or a form field over MAX_FORM_MEMORY_SIZE""" + from flask import jsonify + message = 'The upload is too large. / El archivo es demasiado grande.' + if (request.path.startswith(('/qr/', '/api/')) + or request.headers.get('X-Requested-With') == 'XMLHttpRequest'): + return jsonify({'success': False, 'message': message}), 413 + flash(message, 'error') + referrer = request.referrer or '' + if not referrer.startswith(request.host_url): + referrer = url_for('dashboard.dashboard') + return redirect(referrer) + # ------------------------------------------------------------------ # Startup initialization (runs under gunicorn and flask run alike) # ------------------------------------------------------------------ @@ -395,22 +417,24 @@ def update_existing_qr_codes(): if not qr_codes: return - # Build a base URL that does not require an active request context. - host = os.environ.get('FLASK_HOST', '0.0.0.0') - # 0.0.0.0 is a bind address, not a reachable hostname — default to localhost - if host in ('0.0.0.0', ''): - host = 'localhost' - port = os.environ.get('FLASK_PORT', '5000') - scheme = 'https' if _Cfg.SESSION_COOKIE_SECURE else 'http' - base_url = f"{scheme}://{host}:{port}/" + # Public base URL for QR images generated here. There is no request at + # startup, so it can only come from QR_BASE_URL (e.g. + # https://qr.govservicesinc.com). Without it no image is generated: the + # old FLASK_HOST/FLASK_PORT fallback printed http://localhost:5000/... + # into QR codes that no phone can open. Routes still build images from + # the request when a QR code is created or its styling is edited. + base_url = (_Cfg.QR_BASE_URL or '').rstrip('/') + base_url = f"{base_url}/" if base_url else '' + if not base_url: + lh.logger.info("Startup: QR_BASE_URL not set — missing QR images are not generated at startup") updated_count = 0 for qr_code in qr_codes: - if not qr_code.qr_url or not qr_code.qr_code_image: + if not qr_code.qr_url or (not qr_code.qr_code_image and base_url): try: if not qr_code.qr_url: qr_code.qr_url = generate_qr_url(qr_code.name, qr_code.id) - if not qr_code.qr_code_image: + if not qr_code.qr_code_image and base_url: qr_data = f"{base_url}qr/{qr_code.qr_url}" styling = get_qr_styling(qr_code) qr_code.qr_code_image = generate_qr_code( diff --git a/config.py b/config.py index eb7af30..4d481de 100644 --- a/config.py +++ b/config.py @@ -83,6 +83,17 @@ class Config: os.environ.get('VERIFICATION_PHOTO_MAX_SIZE', str(5 * 1024 * 1024)) ) + # ------------------------------------------------------------------ # + # Request size limits + # ------------------------------------------------------------------ # + # Flask 3.1 rejects any non-file form field larger than MAX_FORM_MEMORY_SIZE + # (default 500 KB) with HTTP 413. The verification photo is sent as a base64 + # TEXT field, so this follows the photo limit (+1 MB for the other fields). + MAX_FORM_MEMORY_SIZE = VERIFICATION_PHOTO_MAX_SIZE + 1024 * 1024 + # Whole-request cap (Excel imports, photos). Nginx client_max_body_size + # must be at least this, or Nginx rejects larger uploads first. + MAX_CONTENT_LENGTH = int(os.environ.get('MAX_UPLOAD_SIZE_MB', '50')) * 1024 * 1024 + # ------------------------------------------------------------------ # # Check-in interval # ------------------------------------------------------------------ # @@ -106,6 +117,15 @@ class Config: FLASK_PORT = int(os.environ.get('FLASK_PORT', '5000')) THREADED = os.environ.get('THREADED', 'True').lower() == 'true' + # Number of reverse proxies in front of the app (Nginx = 1; Cloudflare in + # front of Nginx = 2). ProxyFix reads the real client IP from that many + # X-Forwarded-For hops. 0 = not behind a proxy (local development only). + TRUSTED_PROXY_COUNT = int(os.environ.get('TRUSTED_PROXY_COUNT', '1')) + + # Public base URL of this deployment (e.g. https://qr.govservicesinc.com). + # Used for QR images generated at startup, where there is no request. + QR_BASE_URL = os.environ.get('QR_BASE_URL', '').strip() + # ------------------------------------------------------------------ # # Default admin (used only on first boot) # ------------------------------------------------------------------ # diff --git a/legacy_attendance_service.py b/legacy_attendance_service.py index b3a4835..a97e8d5 100644 --- a/legacy_attendance_service.py +++ b/legacy_attendance_service.py @@ -30,6 +30,7 @@ import pymysql import pymysql.cursors from config import Config +from utils.excel_safety import neutralize_unexpected_formulas try: import openpyxl @@ -323,6 +324,7 @@ def build_legacy_export_workbook(rows): ws.freeze_panes = 'A2' buffer = io.BytesIO() + neutralize_unexpected_formulas(wb) # formula-injection guard (utils/excel_safety.py) wb.save(buffer) buffer.seek(0) return buffer diff --git a/location_logging.py b/location_logging.py index a966008..4668463 100644 --- a/location_logging.py +++ b/location_logging.py @@ -1,7 +1,7 @@ # File: location_logging.py # Enhanced location action logging for Android debugging -from flask import request, jsonify +from flask import request, jsonify, session from datetime import datetime import json import traceback @@ -69,8 +69,12 @@ def create_location_logging_routes(app, db, logger_handler): @app.route('/api/location-debug-info', methods=['GET']) def get_location_debug_info(): """ - Get debugging information about location services + Get debugging information about location services (admins only) """ + # Not public: it used to echo every request header to anyone, including + # the session Cookie, which would defeat HttpOnly if an XSS ever landed. + if session.get('role') != 'admin': + return jsonify({'error': 'Access denied'}), 403 try: user_agent = request.headers.get('User-Agent', '') device_info = extract_device_info(user_agent) @@ -79,7 +83,7 @@ def create_location_logging_routes(app, db, logger_handler): 'timestamp': datetime.now().isoformat(), 'ip_address': get_client_ip_enhanced(), 'device_info': device_info, - 'headers': dict(request.headers), + # request headers are deliberately not echoed (they carry the session Cookie) 'is_android': 'android' in user_agent.lower(), 'is_chrome': 'chrome' in user_agent.lower() and 'edg' not in user_agent.lower(), 'is_secure': request.is_secure, diff --git a/routes/attendance.py b/routes/attendance.py index 1da22ec..161ebee 100644 --- a/routes/attendance.py +++ b/routes/attendance.py @@ -281,7 +281,7 @@ def attendance_report(): CONCAT(e.firstName, ' ', e.lastName) as employee_name, ad.verification_required, ad.verification_status, - ad.verification_photo, + NULL AS verification_photo, -- base64 image, never shown on this page COALESCE(ad.is_dynamic_qr, 0) as is_dynamic_qr FROM attendance_data ad LEFT JOIN qr_codes qc ON ad.qr_code_id = qc.id @@ -310,7 +310,7 @@ def attendance_report(): CONCAT(e.firstName, ' ', e.lastName) as employee_name, ad.verification_required, ad.verification_status, - ad.verification_photo, + NULL AS verification_photo, -- base64 image, never shown on this page COALESCE(ad.is_dynamic_qr, 0) as is_dynamic_qr FROM attendance_data ad LEFT JOIN qr_codes qc ON ad.qr_code_id = qc.id diff --git a/routes/attendance_export.py b/routes/attendance_export.py index 4f10d5c..2ffbeb9 100644 --- a/routes/attendance_export.py +++ b/routes/attendance_export.py @@ -32,6 +32,7 @@ from utils.helpers import ( staff_or_admin_required) from utils.geocoding import (calculate_location_accuracy_enhanced, process_location_data_enhanced, check_location_accuracy_column_exists) +from utils.excel_safety import excel_hyperlink, neutralize_unexpected_formulas import openpyxl from openpyxl.styles import Font, PatternFill, Alignment, Border, Side from openpyxl.utils import get_column_letter @@ -462,7 +463,7 @@ def create_excel_export(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(qr_record.address_latitude):.10f}" lng_formatted = f"{float(qr_record.address_longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added QR address hyperlink for employee {attendance_record.employee_id}") else: @@ -475,7 +476,7 @@ def create_excel_export(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(attendance_record.latitude):.10f}" lng_formatted = f"{float(attendance_record.longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added check-in address hyperlink for employee {attendance_record.employee_id}") else: @@ -488,7 +489,7 @@ def create_excel_export(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(attendance_record.latitude):.10f}" lng_formatted = f"{float(attendance_record.longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added check-in address hyperlink (fallback) for employee {attendance_record.employee_id}") else: @@ -500,7 +501,7 @@ def create_excel_export(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(attendance_record.latitude):.10f}" lng_formatted = f"{float(attendance_record.longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added check-in address hyperlink (no accuracy data) for employee {attendance_record.employee_id}") else: @@ -585,6 +586,7 @@ def create_excel_export(selected_columns, column_names, filters): # Save to BytesIO excel_buffer = io.BytesIO() + neutralize_unexpected_formulas(wb) # formula-injection guard (utils/excel_safety.py) wb.save(excel_buffer) excel_buffer.seek(0) @@ -801,7 +803,7 @@ def create_excel_export_ordered(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(qr_record.address_latitude):.10f}" lng_formatted = f"{float(qr_record.address_longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added QR address hyperlink for employee {attendance_record.employee_id}") else: @@ -814,7 +816,7 @@ def create_excel_export_ordered(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(attendance_record.latitude):.10f}" lng_formatted = f"{float(attendance_record.longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added check-in address hyperlink for employee {attendance_record.employee_id}") else: @@ -827,7 +829,7 @@ def create_excel_export_ordered(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(attendance_record.latitude):.10f}" lng_formatted = f"{float(attendance_record.longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added check-in address hyperlink (fallback) for employee {attendance_record.employee_id}") else: @@ -839,7 +841,7 @@ def create_excel_export_ordered(selected_columns, column_names, filters): # Format coordinates with 10 decimal places lat_formatted = f"{float(attendance_record.latitude):.10f}" lng_formatted = f"{float(attendance_record.longitude):.10f}" - hyperlink_formula = f'=HYPERLINK("http://maps.google.com/maps?q={lat_formatted},{lng_formatted}","{address_text.strip()}")' + hyperlink_formula = excel_hyperlink(f"http://maps.google.com/maps?q={lat_formatted},{lng_formatted}", address_text.strip()) cell.value = hyperlink_formula logger_handler.logger.debug(f"Added check-in address hyperlink (no accuracy data) for employee {attendance_record.employee_id}") else: @@ -933,6 +935,7 @@ def create_excel_export_ordered(selected_columns, column_names, filters): # Save to BytesIO excel_buffer = io.BytesIO() + neutralize_unexpected_formulas(wb) # formula-injection guard (utils/excel_safety.py) wb.save(excel_buffer) excel_buffer.seek(0) diff --git a/routes/auth.py b/routes/auth.py index 2f031e8..7beb9c9 100644 --- a/routes/auth.py +++ b/routes/auth.py @@ -86,10 +86,11 @@ def login(): flash('Please enter both username and password.', 'error') return render_template('login.html') - # Rate-limit check — blocks IPs with 5+ failed attempts in 15 minutes + # Rate-limit check — 5 failures for this IP + username, or 20 for the + # username from any IP, within 15 minutes (see SecurityManager) from flask import current_app sec_mgr = getattr(current_app, 'security_manager', None) - if sec_mgr and sec_mgr.is_auth_rate_limited(): + if sec_mgr and sec_mgr.is_auth_rate_limited(username): logger_handler.log_security_event( event_type="login_rate_limited", description=f"Login blocked by rate limiter for username: {username}", diff --git a/routes/employees.py b/routes/employees.py index ec1375c..04dbc2b 100644 --- a/routes/employees.py +++ b/routes/employees.py @@ -17,6 +17,8 @@ from models.qrcode import QRCode from models.user import User from logger_handler import log_user_activity, log_database_operations from utils.helpers import ( + PAYROLL_AREA_ROLES, + restrict_blueprint_to_roles, admin_required, has_admin_privileges, has_staff_level_access, @@ -25,6 +27,9 @@ from utils.helpers import ( bp = Blueprint('employees', __name__) +# Employee records (sidebar: admin, payroll, accounting) — checked for every route. +restrict_blueprint_to_roles(bp, PAYROLL_AREA_ROLES) + @bp.route('/employees', endpoint='employees') diff --git a/routes/legacy_attendance.py b/routes/legacy_attendance.py index 0e460a3..3a0da00 100644 --- a/routes/legacy_attendance.py +++ b/routes/legacy_attendance.py @@ -15,7 +15,7 @@ from datetime import datetime from extensions import logger_handler from logger_handler import log_user_activity -from utils.helpers import login_required +from utils.helpers import login_required, restrict_blueprint_to_roles, PAYROLL_AREA_ROLES from legacy_attendance_service import ( LegacyDbUnavailable, get_legacy_dashboard_stats, @@ -27,6 +27,9 @@ from legacy_attendance_service import ( bp = Blueprint('legacy_attendance', __name__) +# Legacy Attendance (sidebar: admin, payroll, accounting) — checked for every route. +restrict_blueprint_to_roles(bp, PAYROLL_AREA_ROLES) + # Fixed dropdown values — confirmed values stored in the legacy `records.type` column LEGACY_RECORD_TYPES = ['CHECK IN', 'CHECK OUT'] diff --git a/routes/qr_codes.py b/routes/qr_codes.py index 85a8a6a..1619ca7 100644 --- a/routes/qr_codes.py +++ b/routes/qr_codes.py @@ -20,6 +20,8 @@ from werkzeug.utils import secure_filename from logger_handler import log_user_activity, log_database_operations from sqlalchemy import or_ from utils.helpers import ( + QR_MANAGEMENT_ROLES, + roles_required, admin_required, employee_id_regex_condition, expand_employee_id_filter, @@ -106,6 +108,7 @@ def get_unique_qr_locations(): @bp.route('/qr-codes/create', methods=['GET', 'POST'], endpoint='create_qr_code') +@roles_required(*QR_MANAGEMENT_ROLES) # not project managers (reports only) @login_required @log_database_operations('qr_code_creation') def create_qr_code(): @@ -286,6 +289,7 @@ def create_qr_code(): return render_template('create_qr_code.html', projects=projects, styles=styles) @bp.route('/qr-codes/bulk-import', methods=['GET', 'POST'], endpoint='import_bulk_qr_codes') +@roles_required(*QR_MANAGEMENT_ROLES) @login_required @log_database_operations('qr_code_bulk_import') def import_bulk_qr_codes(): @@ -400,6 +404,7 @@ def import_bulk_qr_codes(): @bp.route('/qr-codes/bulk-import/template', endpoint='download_qr_import_template') +@roles_required(*QR_MANAGEMENT_ROLES) @login_required def download_qr_import_template(): """Download Excel template for bulk QR code import""" @@ -465,6 +470,7 @@ def download_qr_import_template(): return redirect(url_for('qr_codes.import_bulk_qr_codes')) @bp.route('/qr-codes//edit', methods=['GET', 'POST'], endpoint='edit_qr_code') +@roles_required(*QR_MANAGEMENT_ROLES) @login_required @log_database_operations('qr_code_edit') def edit_qr_code(qr_id): @@ -744,19 +750,23 @@ def qr_checkin(qr_url): # Get and validate employee ID employee_id = request.form.get('employee_id', '').strip() - # At most 4 digits — counted on the base ID, so an old-style typed - # suffix ("1234SP") from a page cached before the numeric-only rule - # still passes. Refused, never truncated: a shorter ID is another person. + # 1 to 4 digits, numbers only — checked on the base ID, so an old-style + # typed suffix ("1234SP") from a page cached before the numeric-only rule + # still passes. Anything else is refused, never truncated (a shorter ID + # is another person). This also keeps text such as "=HYPERLINK(...)" out + # of attendance_data and every Excel export built from it. if employee_id: base_for_length, _ = parse_employee_id_for_work_type(employee_id) - if len(re.sub(r'\D', '', base_for_length)) > CHECKIN_EMPLOYEE_ID_MAX_DIGITS: + if (not re.fullmatch(r'[0-9]+', base_for_length) + or len(base_for_length) > CHECKIN_EMPLOYEE_ID_MAX_DIGITS): logger_handler.logger.warning( - f"Check-in rejected: employee ID '{employee_id}' has more than " + f"Check-in rejected: employee ID {employee_id!r} is not 1-" f"{CHECKIN_EMPLOYEE_ID_MAX_DIGITS} digits (QR {qr_url})" ) return jsonify({ 'success': False, - 'message': 'Employee ID must be 4 digits or fewer. / El ID de empleado debe tener 4 dígitos o menos.' + 'message': ('Employee ID must be 1 to 4 digits, numbers only. / ' + 'El ID de empleado debe tener de 1 a 4 dígitos, solo números.') }), 400 # --- ADDED: type of work selected on the check-in page --- @@ -1325,6 +1335,7 @@ def qr_last_work_type(qr_url): @bp.route('/qr-codes//toggle-status', methods=['POST'], endpoint='toggle_qr_status') +@roles_required('admin') # the dashboard shows Activate/Deactivate to admins only @login_required def toggle_qr_status(qr_id): """Toggle QR code active/inactive status""" @@ -1406,6 +1417,7 @@ def open_qr_link(qr_id): }), 500 @bp.route('/qr-codes//activate', methods=['POST'], endpoint='activate_qr_code') +@roles_required('admin') @login_required def activate_qr_code(qr_id): """Activate a QR code""" @@ -1433,6 +1445,7 @@ def activate_qr_code(qr_id): }), 500 @bp.route('/qr-codes//deactivate', methods=['POST'], endpoint='deactivate_qr_code') +@roles_required('admin') @login_required def deactivate_qr_code(qr_id): """Deactivate a QR code""" diff --git a/routes/statistics.py b/routes/statistics.py index 2b26ee4..2ef72bf 100644 --- a/routes/statistics.py +++ b/routes/statistics.py @@ -15,10 +15,15 @@ from models.project import Project from models.user import User from sqlalchemy import text from logger_handler import log_user_activity, log_database_operations -from utils.helpers import login_required, staff_or_admin_required +from utils.helpers import login_required, staff_or_admin_required, restrict_blueprint_to_roles, PAYROLL_AREA_ROLES +from utils.excel_safety import csv_safe bp = Blueprint('statistics', __name__) +# Company-wide statistics: admins in the sidebar; the export already allowed +# payroll and accounting, so the same three roles are checked for every route. +restrict_blueprint_to_roles(bp, PAYROLL_AREA_ROLES) + @bp.route('/statistics', endpoint='qr_statistics') @@ -278,7 +283,8 @@ def export_statistics(): # Write data for row in export_data: - writer.writerow([ + # csv_safe: device / address text starting with = + - @ must not run as a formula + writer.writerow([csv_safe(v) for v in [ row.id, row.employee_id, row.employee_name, str(row.check_in_date), str(row.check_in_time), row.qr_code_name, row.qr_location, row.location_event, @@ -287,7 +293,7 @@ def export_statistics(): row.latitude or '', row.longitude or '', row.address or '', row.location_name or '', str(row.created_timestamp) - ]) + ]]) output.seek(0) diff --git a/routes/time_attendance.py b/routes/time_attendance.py index f5c7268..c134b3e 100644 --- a/routes/time_attendance.py +++ b/routes/time_attendance.py @@ -24,6 +24,8 @@ from sqlalchemy import text, or_ from werkzeug.utils import secure_filename from logger_handler import log_user_activity, log_database_operations from utils.helpers import ( + PAYROLL_AREA_ROLES, + restrict_blueprint_to_roles, admin_required, employee_id_regex_condition, expand_employee_id_filter, @@ -41,6 +43,10 @@ import openpyxl.cell.cell bp = Blueprint('time_attendance', __name__) +# Time Attendance is a payroll area (sidebar: admin, payroll, accounting). +# Checked for EVERY route here — imports and exports feed payroll. +restrict_blueprint_to_roles(bp, PAYROLL_AREA_ROLES) + def build_time_attendance_employee_filter(employee_ids): """ diff --git a/routes/time_attendance_export.py b/routes/time_attendance_export.py index 5f71191..ef06529 100644 --- a/routes/time_attendance_export.py +++ b/routes/time_attendance_export.py @@ -23,6 +23,7 @@ from models.qrcode import QRCode from models.time_attendance import TimeAttendance from sqlalchemy import text from working_hours_calculator import WorkingHoursCalculator, round_time_to_quarter_hour, convert_minutes_to_base100, round_base100_hours +from utils.excel_safety import excel_hyperlink, neutralize_unexpected_formulas import openpyxl from openpyxl.styles import Font, PatternFill, Alignment, Border, Side, numbers from openpyxl.utils import get_column_letter @@ -1377,6 +1378,7 @@ def export_time_attendance_excel(records, project_name_for_filename, date_range_ # Save to BytesIO output = io.BytesIO() + neutralize_unexpected_formulas(wb) # formula-injection guard (utils/excel_safety.py) wb.save(output) output.seek(0) @@ -2237,13 +2239,13 @@ def export_time_attendance_by_building_excel(records, project_name_for_filename, building_address = ref_record.event_description or '' if building_address: encoded_addr = building_address.replace(' ', '+').replace(',', '%2C') - hyperlink_formula = f'=HYPERLINK("https://www.google.com/maps/place/{encoded_addr}","{building_address}")' + hyperlink_formula = excel_hyperlink(f"https://www.google.com/maps/place/{encoded_addr}", building_address) ws.cell(row=current_row, column=11, value=hyperlink_formula) recorded_addr = ref_record.recorded_address or '' if recorded_addr: encoded_recorded = recorded_addr.replace(' ', '+').replace(',', '%2C') - recorded_hyperlink = f'=HYPERLINK("https://www.google.com/maps/place/{encoded_recorded}","{recorded_addr}")' + recorded_hyperlink = excel_hyperlink(f"https://www.google.com/maps/place/{encoded_recorded}", recorded_addr) ws.cell(row=current_row, column=12, value=recorded_hyperlink) current_row += 1 @@ -2363,6 +2365,7 @@ def export_time_attendance_by_building_excel(records, project_name_for_filename, # Save to BytesIO output = io.BytesIO() + neutralize_unexpected_formulas(wb) # formula-injection guard (utils/excel_safety.py) wb.save(output) output.seek(0) diff --git a/routes/users.py b/routes/users.py index e8f098e..a91825e 100644 --- a/routes/users.py +++ b/routes/users.py @@ -201,7 +201,8 @@ def create_user(): flash('Error loading form. Please try again.', 'error') return redirect(url_for('users.users')) -@bp.route('/users//delete', methods=['GET', 'POST'], endpoint='delete_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//delete', methods=['POST'], endpoint='delete_user') @admin_required def delete_user(user_id): """Deactivate user (Admin only) - Fixed with proper validation""" @@ -242,7 +243,8 @@ def delete_user(user_id): flash('Error deactivating user. Please try again.', 'error') return redirect(url_for('users.users')) -@bp.route('/users//reactivate', methods=['GET', 'POST'], endpoint='reactivate_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//reactivate', methods=['POST'], endpoint='reactivate_user') @admin_required def reactivate_user(user_id): """Reactivate a deactivated user (Admin only)""" @@ -272,7 +274,8 @@ def reactivate_user(user_id): flash('Error reactivating user. Please try again.', 'error') return redirect(url_for('users.users')) -@bp.route('/users//promote', methods=['GET', 'POST'], endpoint='promote_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//promote', methods=['POST'], endpoint='promote_user') @admin_required def promote_user(user_id): """Promote a staff user to admin (Admin only)""" @@ -302,7 +305,8 @@ def promote_user(user_id): flash('Error promoting user. Please try again.', 'error') return redirect(url_for('users.users')) -@bp.route('/users//demote', methods=['GET', 'POST'], endpoint='demote_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//demote', methods=['POST'], endpoint='demote_user') @admin_required def demote_user(user_id): """Demote an admin user to staff (Admin only)""" @@ -616,7 +620,8 @@ def toggle_user_status(user_id): 'message': 'Error updating user status. Please try again.' }), 500 -@bp.route('/users//activate', methods=['GET', 'POST'], endpoint='activate_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//activate', methods=['POST'], endpoint='activate_user') @admin_required def activate_user(user_id): """Activate a user (Admin only) - Alternative route""" @@ -649,7 +654,8 @@ def activate_user(user_id): flash('Error activating user. Please try again.', 'error') return redirect(url_for('users.users')) -@bp.route('/users//deactivate', methods=['GET', 'POST'], endpoint='deactivate_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//deactivate', methods=['POST'], endpoint='deactivate_user') @admin_required def deactivate_user(user_id): """Deactivate a user (Admin only) - Alternative route""" @@ -900,7 +906,8 @@ def reverse_geocode_api(): 'message': 'Internal server error during reverse geocoding. Please try again.' }), 500 -@bp.route('/users//permanently-delete', methods=['GET', 'POST'], endpoint='permanently_delete_user') +# POST only: a state change must carry a CSRF token — a GET link could be forged +@bp.route('/users//permanently-delete', methods=['POST'], endpoint='permanently_delete_user') @admin_required def permanently_delete_user(user_id): """Permanently delete user but preserve associated QR codes (Admin only)""" diff --git a/static/js/qr_destination.js b/static/js/qr_destination.js index d631153..b64a998 100644 --- a/static/js/qr_destination.js +++ b/static/js/qr_destination.js @@ -680,16 +680,28 @@ function handleCheckinError(error) { } function updateSubmitButton(isLoading) { - const submitButton = document.getElementById("submitCheckin"); + // The check-in button's id is "submitButton" (qr_destination.html). Looking + // only for the old "submitCheckin" id found nothing, so after the form swap + // at 1.5 s the button never showed "Processing" and employees tapped again. + const submitButton = + document.getElementById("submitCheckin") || + document.getElementById("submitButton"); if (submitButton) { if (isLoading) { submitButton.disabled = true; submitButton.innerHTML = - ' Processing...'; + '' + + 'Processing' + + '/' + + 'Procesando'; } else { submitButton.disabled = false; - submitButton.innerHTML = - ' Submit'; + if (typeof renderSubmitButton === "function") { + renderSubmitButton(); // the page's own Check In / Check Out label + } else { + submitButton.innerHTML = + ' Submit'; + } } applyTranslations(); } diff --git a/static/js/users.js b/static/js/users.js index 058bb87..e1e9780 100644 --- a/static/js/users.js +++ b/static/js/users.js @@ -220,7 +220,7 @@ class UsersManager { try { const response = await fetch(`/users/${userId}/delete`, { - method: "GET", + method: "POST", headers: { "X-Requested-With": "XMLHttpRequest", 'X-CSRF-Token': (window.qrConfig && window.qrConfig.csrfToken) || '', @@ -246,7 +246,7 @@ class UsersManager { async reactivateUser(userId, userName) { try { const response = await fetch(`/users/${userId}/reactivate`, { - method: "GET", + method: "POST", headers: { "X-Requested-With": "XMLHttpRequest", 'X-CSRF-Token': (window.qrConfig && window.qrConfig.csrfToken) || '', @@ -279,7 +279,7 @@ class UsersManager { try { const response = await fetch(`/users/${userId}/promote`, { - method: "GET", + method: "POST", headers: { "X-Requested-With": "XMLHttpRequest", 'X-CSRF-Token': (window.qrConfig && window.qrConfig.csrfToken) || '', @@ -312,7 +312,7 @@ class UsersManager { try { const response = await fetch(`/users/${userId}/demote`, { - method: "GET", + method: "POST", headers: { "X-Requested-With": "XMLHttpRequest", 'X-CSRF-Token': (window.qrConfig && window.qrConfig.csrfToken) || '', diff --git a/templates/users.html b/templates/users.html index df1810d..807acc8 100644 --- a/templates/users.html +++ b/templates/users.html @@ -289,27 +289,33 @@ Code Management{% endblock %} {% block extra_head %} {% if user.role != 'admin' %} - - - + + + {% else %} {% if users|selectattr('role', 'equalto', 'admin')|selectattr('active_status', 'equalto', True)|list|length > 1 %} - - - + + + {% else %}