From 03ce083691ab33530fc038e7cc256f95ff9432f3 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 4 Jul 2026 15:49:47 -0400 Subject: [PATCH] July 4 - Update security --- CLAUDE.md | 3 +- app/__init__.py | 10 +++++ app/routes/signup.py | 4 +- app/utils/forms.py | 52 +++++++++++++++++++---- control/panel/__init__.py | 6 +++ tests/test_security_hardening.py | 73 ++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 tests/test_security_hardening.py diff --git a/CLAUDE.md b/CLAUDE.md index f24dff5..923dabe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1259,7 +1259,7 @@ set -a; . /etc/jqc/control.env; set +a | 48 | **`upload_photo_ajax` endpoint on inspections blueprint** | `POST //upload-photo` with `@limiter.limit("30 per minute")`. Triggers on file selection (not form submit) so photos survive AJAX draft-save and page navigation. Stores in `inspection_photos/` subfolder; returns `{ok, path}`. | | 49 | **Template schema snapshotted at submit time** | `execute()` POST stores `form_fields` list as `_template_schema` inside `inspection.notes` JSON. `view()` prefers this snapshot over the live template so historical inspection views remain correct if the template changes later. | | 50 | **`mobile_local_id` UUID format validation on write endpoints** | `POST /api/v1/inspections` and `POST /api/v1/issues` validate `mobile_local_id` against `_UUID_RE` regex. Rejects non-UUID strings with HTTP 400. Prevents garbage values from being stored as idempotency keys. | -| 51 | **Security response headers via `@app.after_request`** | Added `X-Content-Type-Options: nosniff`, `X-Frame-Options: SAMEORIGIN`, `Referrer-Policy: strict-origin-when-cross-origin`, and a `Content-Security-Policy` (CDN allowlist + `unsafe-inline`). Uses `setdefault` so API responses can override if needed. | +| 51 | **Security response headers via `@app.after_request`** | Sets `X-Content-Type-Options: nosniff`, `X-Frame-Options: SAMEORIGIN`, `Referrer-Policy: strict-origin-when-cross-origin`, and a `Content-Security-Policy`. CSP includes `object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'` (all forms post same-origin, so `form-action 'self'` is safe). `script-src`/`style-src` still carry `'unsafe-inline'` — removing that needs a nonce migration across all inline scripts (larger follow-up, not done). **HSTS** (`Strict-Transport-Security: max-age=31536000`) is emitted only when the request is HTTPS (`request.is_secure` or `X-Forwarded-Proto: https`); `includeSubDomains` is intentionally omitted so a tenant custom domain never force-upgrades an unrelated customer subdomain. The panel (`control/panel/__init__.py`) mirrors these. Uses `setdefault` so API responses can override. | | 52 | **Inspection `execute.html` offline-resilient photo flow** | Photos are uploaded immediately on file selection via `uploadPhotoField()` (XHR to `upload_photo_ajax`). Server path is stored in ``. AJAX draft-save and flag-issue submission read these hidden fields so photos are never lost on navigation. | | 53 | **Flag-issue panel is an offcanvas — not a page navigation** | Converted from a navigate-away flow to a Bootstrap offcanvas. Draft is saved first via `saveDraft()`, then the flag-issue form is submitted via `fetch()` FormData, then the page reloads. Eliminates the entire class of "photos lost on navigation" bugs. | | 54 | **Bulk issue verification via `POST /issues/bulk-verify`** | `@supervisor_required`. Accepts `issue_ids` list from form. Skips issues not in `resolved` or `pending_verification` state. Calls `log_action()` after `db.session.commit()` per rule 10. | @@ -1289,6 +1289,7 @@ set -a; . /etc/jqc/control.env; set +a | 85 | **Apex host serves the public landing page; the landing route lives at `/welcome`, NOT `/`** | The dashboard owns `/` (login-gated) on tenant hosts, so the landing page cannot register a second `/` route (same collision class as rule 84). Instead the tenant middleware detects the apex host (`TENANT_BASE_DOMAIN` + `www.`) and calls `landing.index` directly for `/`, redirecting other non-exempt apex paths to `/`. `/welcome`, `/signup`, `/static/` are tenant-exempt. **The apex check runs BEFORE the `MULTI_TENANT_ENABLED` gate** — it must work in single-tenant mode too, otherwise the app serves its default database (tenant-zero) for the apex host and the landing page never shows. Requires the Nginx apex block to **proxy** (not 301-redirect) to port 8000 with `Host` passed through, and `TENANT_BASE_DOMAIN` set correctly in the app environment. | | 86 | **Free plan is free-forever, not a trial** | `signup.index()` passes `trial_days=0` for `plan_code == 'free'`; `create_tenant()` then sets `subscription_status='active'` (no `trial_ends_at`) so `_billing_gate()` never blocks it. Paid plans keep the 14-day trial (`trial_days=14`). The welcome email adapts via `trial_note` and hides the trial row when `trial_ends_at` is blank. Do not reintroduce a hardcoded `trial_days=14` in the signup path. | | 87 | **MFA is opt-in, TOTP-based, with hashed one-time recovery codes** | `app/utils/mfa.py` (data plane) and `control/mfa.py` (panel) are pure-logic mirrors — keep them in sync (same rule class as `time_utils`). The login challenge (`/auth/mfa`, panel `/mfa`) fires for ANY account with `mfa_enabled=1`; `login_user()`/`session['sa_id']` is deferred until the code passes. Recovery codes are stored ONLY as werkzeug hashes and are single-use (consumed on match). Disable requires a current TOTP code OR the password. **Lock-out escape hatch:** because MFA is per-account opt-in, the recovery path is the primary unlock; the operational last resort is a DB update `UPDATE users SET mfa_enabled=0, mfa_secret=NULL, mfa_recovery_codes=NULL WHERE username=...` (or the same on `superadmins`). Do not store `mfa_secret`/recovery codes in plaintext, and do not skip the deferred-login pattern. | +| 88 | **Password strength enforced by one shared `strong_password()` validator** | Lives in `app/utils/forms.py`: ≥8 chars, at least one letter AND one digit, and not in a small common-password blocklist. Applied to every password-setting form — `ProfileForm`, `UserForm`, `CustomerForm`, `ResetPasswordForm`, `SetPasswordForm`, and `signup.SignupForm` (imports it). Sits after `Optional()` on edit forms (skips blank = "leave unchanged"). Do not re-introduce ad-hoc `Length(min=6)` password rules — route new password fields through `strong_password()` so the policy stays consistent. | --- diff --git a/app/__init__.py b/app/__init__.py index 75aefca..f3121a0 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -267,6 +267,7 @@ def create_app(config_name='default'): # obvious XSS vectors without breaking Bootstrap CDN / Google Fonts. @app.after_request def set_security_headers(response): + from flask import request as _request response.headers.setdefault('X-Content-Type-Options', 'nosniff') response.headers.setdefault('X-Frame-Options', 'SAMEORIGIN') response.headers.setdefault('Referrer-Policy', 'strict-origin-when-cross-origin') @@ -279,8 +280,17 @@ def create_app(config_name='default'): "img-src 'self' data: blob: https://maps.gstatic.com https://maps.googleapis.com; " "connect-src 'self' https://cdn.jsdelivr.net; " "frame-src https://maps.google.com https://www.google.com; " + # Hardening directives that don't affect existing inline scripts/styles: + # block plugins, injected tags, and cross-origin form posts. + "object-src 'none'; base-uri 'self'; form-action 'self'; " "frame-ancestors 'none';" ) + # HSTS — advertise only over HTTPS (Nginx terminates TLS and forwards + # X-Forwarded-Proto). includeSubDomains is deliberately OMITTED: a tenant + # custom domain may run unrelated subdomains that are not yet HTTPS, and + # this header must never force-upgrade one of those. + if _request.is_secure or _request.headers.get('X-Forwarded-Proto', '') == 'https': + response.headers.setdefault('Strict-Transport-Security', 'max-age=31536000') return response # ── Error handler: 413 Request Entity Too Large ─────────────────────── diff --git a/app/routes/signup.py b/app/routes/signup.py index 61e5f2a..6f84204 100644 --- a/app/routes/signup.py +++ b/app/routes/signup.py @@ -23,6 +23,7 @@ from flask import Blueprint, render_template, request, flash, redirect, current_ from flask_wtf import FlaskForm from wtforms import StringField, PasswordField, SelectField from wtforms.validators import DataRequired, Email, Length, EqualTo, ValidationError +from app.utils.forms import strong_password bp = Blueprint('signup', __name__, url_prefix='/signup') logger = logging.getLogger(__name__) @@ -92,7 +93,8 @@ class SignupForm(FlaskForm): validators=[DataRequired(), Length(min=2, max=32)]) plan = SelectField('Plan', choices=[]) # populated in view password = PasswordField('Password', - validators=[DataRequired(), Length(min=8, max=128)]) + validators=[DataRequired(), Length(max=128), + strong_password()]) confirm = PasswordField('Confirm Password', validators=[EqualTo('password', 'Passwords must match.')]) diff --git a/app/utils/forms.py b/app/utils/forms.py index 2c51328..5e94ea0 100644 --- a/app/utils/forms.py +++ b/app/utils/forms.py @@ -5,9 +5,46 @@ from wtforms import (StringField, PasswordField, SelectField, TextAreaField, RadioField) from wtforms.validators import (DataRequired, Email, Length, EqualTo, Optional, NumberRange, ValidationError) +import re as _re from app.models.user import User +# ── Password strength ────────────────────────────────────────────────────────── +# A small blocklist of trivially weak passwords, matched case-insensitively. +_COMMON_PASSWORDS = { + 'password', 'password1', 'password123', '12345678', '123456789', + 'qwerty123', 'qwertyui', '11111111', 'letmein1', 'welcome1', 'welcome123', + 'iloveyou', 'admin123', 'changeme1', 'passw0rd', 'abc12345', 'football1', +} + + +def strong_password(min_length=8): + """WTForms validator enforcing a baseline password strength. + + Policy (NIST-aligned — length first, light complexity, blocklist): + * at least ``min_length`` characters, + * at least one letter AND one digit, + * not a well-known weak password. + + Skips empty values, so it can sit after ``Optional()`` on edit forms where a + blank password means "leave the existing one unchanged". + """ + def _validator(form, field): + pw = field.data or '' + if not pw: + return + if len(pw) < min_length: + raise ValidationError( + f'Password must be at least {min_length} characters long.') + if not (_re.search(r'[A-Za-z]', pw) and _re.search(r'\d', pw)): + raise ValidationError( + 'Password must include at least one letter and one number.') + if pw.lower() in _COMMON_PASSWORDS: + raise ValidationError( + 'That password is too common — please choose a less predictable one.') + return _validator + + # ── Auth ───────────────────────────────────────────────────────────────────── class LoginForm(FlaskForm): @@ -21,7 +58,7 @@ class ProfileForm(FlaskForm): full_name = StringField('Full Name', validators=[Optional(), Length(max=150)]) email = StringField('Email', validators=[DataRequired(), Email(), Length(max=255)]) current_password = PasswordField('Current Password', validators=[Optional()]) - new_password = PasswordField('New Password', validators=[Optional(), Length(min=6, max=100)]) + new_password = PasswordField('New Password', validators=[Optional(), Length(max=100), strong_password()]) confirm_password = PasswordField('Confirm New Password', validators=[EqualTo('new_password', message='Passwords must match.')]) def __init__(self, user=None, *args, **kwargs): @@ -48,7 +85,7 @@ class UserForm(FlaskForm): username = StringField('Username', validators=[DataRequired(), Length(min=3, max=100)]) full_name = StringField('Full Name', validators=[Optional(), Length(max=150)]) email = StringField('Email', validators=[DataRequired(), Email(), Length(max=255)]) - password = PasswordField('Password', validators=[Optional(), Length(min=6, max=100)]) + password = PasswordField('Password', validators=[Optional(), Length(max=100), strong_password()]) confirm_password = PasswordField('Confirm Password', validators=[Optional(), EqualTo('password')]) role = SelectField('Role', choices=[ ('admin', 'Administrator'), @@ -67,10 +104,7 @@ class UserForm(FlaskForm): super().__init__(*args, **kwargs) self.user = user - def validate_password(self, field): - """Enforce minimum length only when a new password is actually provided.""" - if field.data and len(field.data) < 6: - raise ValidationError('Password must be at least 6 characters.') + # Password strength is enforced by the strong_password() field validator. def validate_confirm_password(self, field): """Require confirmation to match only when a new password is provided.""" @@ -212,7 +246,7 @@ class CustomerUserForm(FlaskForm): username = StringField('Username', validators=[DataRequired(), Length(min=3, max=100)]) full_name = StringField('Full Name', validators=[Optional(), Length(max=150)]) email = StringField('Email', validators=[DataRequired(), Email(), Length(max=255)]) - password = PasswordField('Password', validators=[Optional(), Length(min=8)]) + password = PasswordField('Password', validators=[Optional(), Length(max=128), strong_password()]) confirm_password = PasswordField('Confirm Password', validators=[Optional(), EqualTo('password', message='Passwords must match.')]) @@ -258,7 +292,7 @@ class ForgotPasswordForm(FlaskForm): class ResetPasswordForm(FlaskForm): - password = PasswordField('New Password', validators=[DataRequired(), Length(min=8, max=100)]) + password = PasswordField('New Password', validators=[DataRequired(), Length(max=100), strong_password()]) confirm_password = PasswordField('Confirm Password', validators=[DataRequired(), EqualTo('password', message='Passwords must match.')]) @@ -266,7 +300,7 @@ class ResetPasswordForm(FlaskForm): class SetPasswordForm(FlaskForm): """Public form for customer to choose their username and password via emailed link.""" username = StringField('Choose a Username', validators=[DataRequired(), Length(min=3, max=100)]) - password = PasswordField('Password', validators=[DataRequired(), Length(min=8, max=100)]) + password = PasswordField('Password', validators=[DataRequired(), Length(max=100), strong_password()]) confirm_password = PasswordField('Confirm Password', validators=[DataRequired(), EqualTo('password', message='Passwords must match.')]) diff --git a/control/panel/__init__.py b/control/panel/__init__.py index 48480c6..7a36089 100644 --- a/control/panel/__init__.py +++ b/control/panel/__init__.py @@ -96,6 +96,7 @@ def create_panel_app(): # ── Security headers ────────────────────────────────────────────────── @app.after_request def security_headers(response): + from flask import request as _request response.headers.setdefault('X-Content-Type-Options', 'nosniff') response.headers.setdefault('X-Frame-Options', 'DENY') response.headers.setdefault('Referrer-Policy', 'strict-origin-when-cross-origin') @@ -106,8 +107,13 @@ def create_panel_app(): "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " "font-src 'self' https://cdn.jsdelivr.net; " "img-src 'self' data:; " + "object-src 'none'; base-uri 'self'; form-action 'self'; " "frame-ancestors 'none';" ) + # HSTS over HTTPS only. The panel serves a single host (admin.jqc.app), + # so includeSubDomains is safe here — but kept off for parity/caution. + if _request.is_secure or _request.headers.get('X-Forwarded-Proto', '') == 'https': + response.headers.setdefault('Strict-Transport-Security', 'max-age=31536000') return response return app diff --git a/tests/test_security_hardening.py b/tests/test_security_hardening.py new file mode 100644 index 0000000..8f769aa --- /dev/null +++ b/tests/test_security_hardening.py @@ -0,0 +1,73 @@ +""" +tests/test_security_hardening.py +-------------------------------- +Covers the security quick-wins: + + * strong_password() validator — length, letter+digit, common-password blocklist + * response security headers — hardened CSP directives + HSTS over HTTPS only +""" + +import pytest +from wtforms.validators import ValidationError + + +# ── Password strength validator ───────────────────────────────────────────── + +class _Field: + def __init__(self, data): + self.data = data + + +def _accepts(pw, **kw): + from app.utils.forms import strong_password + try: + strong_password(**kw)(None, _Field(pw)) + return True + except ValidationError: + return False + + +def test_strong_password_accepts_reasonable(): + assert _accepts('abcd1234') # 8 chars, letter + digit + assert _accepts('Tr0ubador!!') # longer, mixed + assert _accepts('') # empty is skipped (Optional handles required-ness) + + +def test_strong_password_rejects_weak(): + assert not _accepts('short1') # too short (< 8) + assert not _accepts('allletters') # no digit + assert not _accepts('12345678') # no letter + assert not _accepts('password1') # common-password blocklist + assert not _accepts('welcome1') # common-password blocklist + + +def test_strong_password_custom_min_length(): + assert not _accepts('abcd123', min_length=8) # 7 chars + assert _accepts('abcd1234', min_length=8) + + +# ── Response security headers ──────────────────────────────────────────────── + +@pytest.fixture +def client(app): + return app.test_client() + + +def test_hardened_csp_present(client): + resp = client.get('/auth/login') + csp = resp.headers.get('Content-Security-Policy', '') + assert "object-src 'none'" in csp + assert "base-uri 'self'" in csp + assert "form-action 'self'" in csp + assert "frame-ancestors 'none'" in csp + assert resp.headers.get('X-Content-Type-Options') == 'nosniff' + + +def test_hsts_only_over_https(client): + # Plain HTTP request — no HSTS advertised. + http = client.get('/auth/login') + assert 'Strict-Transport-Security' not in http.headers + + # Behind a TLS-terminating proxy (X-Forwarded-Proto=https) — HSTS present. + https = client.get('/auth/login', headers={'X-Forwarded-Proto': 'https'}) + assert https.headers.get('Strict-Transport-Security', '').startswith('max-age=')