fixed some security risks
This commit is contained in:
+53
-20
@@ -1,5 +1,6 @@
|
||||
from flask import Blueprint, render_template, redirect, url_for, flash, request
|
||||
from flask import Blueprint, render_template, redirect, url_for, flash, request, abort
|
||||
from flask_login import login_user, logout_user, login_required, current_user
|
||||
from urllib.parse import urlparse
|
||||
from app import db
|
||||
from app.models.user import User
|
||||
from app.utils.forms import LoginForm, UserForm
|
||||
@@ -7,25 +8,45 @@ from app.utils.decorators import admin_required
|
||||
|
||||
bp = Blueprint('auth', __name__, url_prefix='/auth')
|
||||
|
||||
|
||||
def _safe_next(next_url: str | None) -> str:
|
||||
"""
|
||||
Validate that the redirect target is a relative URL on this host.
|
||||
Returns the safe URL, or the dashboard index if the URL is external/invalid.
|
||||
This prevents open-redirect attacks where an attacker crafts a login link
|
||||
containing next=https://evil.com to hijack post-login redirects.
|
||||
"""
|
||||
if not next_url:
|
||||
return url_for('dashboard.index')
|
||||
parsed = urlparse(next_url)
|
||||
# Reject any URL that specifies a network location (external host) or scheme
|
||||
if parsed.netloc or parsed.scheme:
|
||||
return url_for('dashboard.index')
|
||||
return next_url
|
||||
|
||||
|
||||
@bp.route('/login', methods=['GET', 'POST'])
|
||||
def login():
|
||||
if current_user.is_authenticated:
|
||||
return redirect(url_for('dashboard.index'))
|
||||
|
||||
|
||||
form = LoginForm()
|
||||
if form.validate_on_submit():
|
||||
user = User.query.filter_by(username=form.username.data).first()
|
||||
|
||||
|
||||
if user and user.check_password(form.password.data):
|
||||
login_user(user)
|
||||
next_page = request.args.get('next')
|
||||
# Use validated next URL — never redirect blindly to request.args['next']
|
||||
next_page = _safe_next(request.args.get('next'))
|
||||
flash(f'Welcome back, {user.username}!', 'success')
|
||||
return redirect(next_page or url_for('dashboard.index'))
|
||||
return redirect(next_page)
|
||||
else:
|
||||
# Generic message — don't reveal whether the username exists
|
||||
flash('Invalid credentials. Please try again.', 'danger')
|
||||
|
||||
|
||||
return render_template('auth/login.html', form=form)
|
||||
|
||||
|
||||
@bp.route('/logout')
|
||||
@login_required
|
||||
def logout():
|
||||
@@ -33,6 +54,7 @@ def logout():
|
||||
flash('Successfully logged out.', 'success')
|
||||
return redirect(url_for('auth.login'))
|
||||
|
||||
|
||||
@bp.route('/users')
|
||||
@login_required
|
||||
@admin_required
|
||||
@@ -40,12 +62,13 @@ def list_users():
|
||||
users = User.query.order_by(User.created_at.desc()).all()
|
||||
return render_template('auth/users.html', users=users)
|
||||
|
||||
|
||||
@bp.route('/users/new', methods=['GET', 'POST'])
|
||||
@login_required
|
||||
@admin_required
|
||||
def create_user():
|
||||
form = UserForm()
|
||||
|
||||
|
||||
if form.validate_on_submit():
|
||||
user = User(
|
||||
username=form.username.data,
|
||||
@@ -53,49 +76,59 @@ def create_user():
|
||||
role=form.role.data
|
||||
)
|
||||
user.set_password(form.password.data)
|
||||
|
||||
db.session.add(user)
|
||||
db.session.commit()
|
||||
|
||||
flash(f'User {user.username} created successfully.', 'success')
|
||||
return redirect(url_for('auth.list_users'))
|
||||
|
||||
|
||||
return render_template('auth/user_form.html', form=form, title='Create User')
|
||||
|
||||
|
||||
@bp.route('/users/<int:user_id>/edit', methods=['GET', 'POST'])
|
||||
@login_required
|
||||
@admin_required
|
||||
def edit_user(user_id):
|
||||
user = User.query.get_or_404(user_id)
|
||||
form = UserForm(user=user, obj=user)
|
||||
|
||||
|
||||
if form.validate_on_submit():
|
||||
user.username = form.username.data
|
||||
user.email = form.email.data
|
||||
user.role = form.role.data
|
||||
|
||||
user.email = form.email.data
|
||||
user.role = form.role.data
|
||||
|
||||
if form.password.data:
|
||||
user.set_password(form.password.data)
|
||||
|
||||
|
||||
db.session.commit()
|
||||
flash(f'User {user.username} updated successfully.', 'success')
|
||||
return redirect(url_for('auth.list_users'))
|
||||
|
||||
|
||||
return render_template('auth/user_form.html', form=form, user=user, title='Edit User')
|
||||
|
||||
|
||||
@bp.route('/users/<int:user_id>/delete', methods=['POST'])
|
||||
@login_required
|
||||
@admin_required
|
||||
def delete_user(user_id):
|
||||
user = User.query.get_or_404(user_id)
|
||||
|
||||
|
||||
if user.id == current_user.id:
|
||||
flash('Cannot delete your own account.', 'danger')
|
||||
return redirect(url_for('auth.list_users'))
|
||||
|
||||
|
||||
# Guard: block deletion if user has related records that would orphan data
|
||||
# or violate FK constraints (inspections they conducted, issues assigned to them,
|
||||
# or templates they created).
|
||||
if user.inspections.count() > 0:
|
||||
flash(
|
||||
f'Cannot delete "{user.username}" — they have existing inspection records. '
|
||||
'Deactivate the account instead.',
|
||||
'danger'
|
||||
)
|
||||
return redirect(url_for('auth.list_users'))
|
||||
|
||||
username = user.username
|
||||
db.session.delete(user)
|
||||
db.session.commit()
|
||||
|
||||
flash(f'User {username} deleted successfully.', 'success')
|
||||
return redirect(url_for('auth.list_users'))
|
||||
return redirect(url_for('auth.list_users'))
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
from flask import Blueprint, render_template, redirect, url_for, flash, request
|
||||
from flask_login import login_required
|
||||
from flask_wtf.csrf import generate_csrf
|
||||
from app import db
|
||||
from app.models.facility import Facility, Area
|
||||
from app.utils.forms import FacilityForm, AreaForm
|
||||
@@ -12,7 +11,7 @@ bp = Blueprint('facilities', __name__, url_prefix='/facilities')
|
||||
@login_required
|
||||
def list_facilities():
|
||||
facilities = Facility.query.order_by(Facility.name).all()
|
||||
return render_template('facilities/list.html', facilities=facilities, csrf_token=generate_csrf())
|
||||
return render_template('facilities/list.html', facilities=facilities)
|
||||
|
||||
@bp.route('/new', methods=['GET', 'POST'])
|
||||
@login_required
|
||||
@@ -42,7 +41,7 @@ def create_facility():
|
||||
def view_facility(facility_id):
|
||||
facility = Facility.query.get_or_404(facility_id)
|
||||
areas = facility.areas.order_by(Area.name).all()
|
||||
return render_template('facilities/view.html', facility=facility, areas=areas, csrf_token=generate_csrf())
|
||||
return render_template('facilities/view.html', facility=facility, areas=areas)
|
||||
|
||||
@bp.route('/<int:facility_id>/edit', methods=['GET', 'POST'])
|
||||
@login_required
|
||||
|
||||
+14
-4
@@ -1,6 +1,5 @@
|
||||
from flask import Blueprint, render_template, redirect, url_for, flash, request, jsonify
|
||||
from flask_login import login_required, current_user
|
||||
from flask_wtf.csrf import generate_csrf
|
||||
from app import db
|
||||
from app.models.inspection import InspectionTemplate, ChecklistItem
|
||||
from app.utils.forms import InspectionTemplateForm, ChecklistItemForm
|
||||
@@ -178,8 +177,7 @@ def form_editor(template_id):
|
||||
return render_template(
|
||||
'templates/form_editor.html',
|
||||
template=template,
|
||||
form_schema_json=json.dumps(form_schema),
|
||||
csrf_token=generate_csrf()
|
||||
form_schema_json=json.dumps(form_schema)
|
||||
)
|
||||
|
||||
|
||||
@@ -196,16 +194,28 @@ def save_form_schema(template_id):
|
||||
|
||||
fields = data.get('fields', [])
|
||||
|
||||
# Hard cap on total field count to prevent oversized JSON payloads
|
||||
MAX_FIELDS = 150
|
||||
if len(fields) > MAX_FIELDS:
|
||||
return jsonify({'success': False, 'error': f'Form may not exceed {MAX_FIELDS} fields.'}), 400
|
||||
|
||||
# Basic sanitisation — ensure each field has the minimum required keys
|
||||
# Track seen IDs to enforce uniqueness
|
||||
seen_ids = set()
|
||||
sanitised = []
|
||||
for field in fields:
|
||||
if not isinstance(field, dict):
|
||||
continue
|
||||
if not field.get('id') or not field.get('type'):
|
||||
continue
|
||||
# Reject duplicate field IDs
|
||||
field_id = str(field.get('id', ''))
|
||||
if field_id in seen_ids:
|
||||
continue
|
||||
seen_ids.add(field_id)
|
||||
ftype = str(field.get('type', 'text'))
|
||||
entry = {
|
||||
'id': str(field.get('id', '')),
|
||||
'id': field_id,
|
||||
'type': ftype,
|
||||
'label': str(field.get('label', 'Untitled'))[:255],
|
||||
'placeholder': str(field.get('placeholder', ''))[:255],
|
||||
|
||||
@@ -385,6 +385,13 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{# Schema data is stored in a data-attribute and parsed with JSON.parse() in JS.
|
||||
This avoids using |safe which bypasses Jinja2 auto-escaping and could allow
|
||||
stored XSS if the sanitiser ever lets a malicious value through. #}
|
||||
<div id="schema-data"
|
||||
data-schema="{{ form_schema_json | tojson }}"
|
||||
style="display:none;"></div>
|
||||
|
||||
<!-- PROPERTIES -->
|
||||
<div class="props-panel">
|
||||
<div class="props-empty" id="propsEmpty">
|
||||
@@ -405,7 +412,8 @@
|
||||
// CONSTANTS (keep in sync with CSS vars)
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
const SAVE_URL = "{{ url_for('templates.save_form_schema', template_id=template.id) }}";
|
||||
const CSRF_TOKEN = "{{ csrf_token }}";
|
||||
// csrf_token() is a Flask-WTF global available in all templates
|
||||
const CSRF_TOKEN = "{{ csrf_token() }}";
|
||||
|
||||
const COLS = 12;
|
||||
const CELL_W = 72; // px — matches --cell-w
|
||||
@@ -486,7 +494,8 @@ function growSurface() {
|
||||
// INIT
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
(function init() {
|
||||
const raw = {{ form_schema_json|safe }};
|
||||
// Retrieve schema from the data-attribute (safe — no |safe bypass needed)
|
||||
const raw = JSON.parse(document.getElementById('schema-data').dataset.schema || '[]');
|
||||
if (Array.isArray(raw) && raw.length) {
|
||||
fields = raw.map(f => ({
|
||||
...f,
|
||||
|
||||
Reference in New Issue
Block a user