fix(security): appliquer les schemas de validation sur la gestion des comptes
SEC-AUTHZ-001. CreateUserSchema, EditUserSchema et EditProfileSchema
etaient importes dans users.py et jamais appeles : chaque nom
n'apparaissait qu'une fois dans le fichier, sur sa ligne d'import. Les
trois routes lisaient request.form directement.
Consequences levees :
- aucune politique de mot de passe sur create_user, edit_user et
edit_profile. Un mot de passe d'un caractere etait accepte pour un
compte administrateur.
- aucune validation de format sur username, email, phone,
discord_user_id.
- edit_user ne verifiait pas l'unicite du courriel avant affectation :
la contrainte unique remontait en IntegrityError, donc en erreur 500.
Un controle explicite excluant l'utilisateur courant est ajoute.
C'est aussi le point d'injection de la chaine de XSS stocke SEC-XSS-001 :
edit_profile acceptait n'importe quel nom d'utilisateur, charge HTML
comprise, qui ressortait ensuite en JSON via /matches/api/events et
etait injectee par innerHTML dans le calendrier.
Deux details de formulaire imposaient un adaptateur, _form_payload :
- request.form.to_dict() ne conserve que la premiere valeur d'une cle
repetee, donc games doit etre relu avec getlist().
- une case a cocher non cochee est absente de la soumission, ce qui
n'est pas la meme chose qu'un load_default. Sans injection explicite,
decocher is_active_account aurait cesse de desactiver le compte.
- un mot de passe vide signifie "conserver l'actuel" et non "definir le
mot de passe vide" : le champ est retire avant validation.
Le controle manuel du role devient redondant, le schema le contraint deja
par OneOf(USER_TYPES).
Verifie par quatre tests qui echouaient avant ce changement.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
+98
-36
@@ -7,7 +7,7 @@ import os
|
|||||||
import uuid
|
import uuid
|
||||||
from flask import Blueprint, render_template, redirect, url_for, flash, request, jsonify, send_file
|
from flask import Blueprint, render_template, redirect, url_for, flash, request, jsonify, send_file
|
||||||
from flask_login import login_required, current_user
|
from flask_login import login_required, current_user
|
||||||
from app.extensions import db, hash_password, csrf
|
from app.extensions import db, hash_password
|
||||||
from app.models import (
|
from app.models import (
|
||||||
Admin, Manager, Coach, Player, Scout,
|
Admin, Manager, Coach, Player, Scout,
|
||||||
User, USER_TYPES, ESPORT_GAMES,
|
User, USER_TYPES, ESPORT_GAMES,
|
||||||
@@ -18,11 +18,11 @@ from app.models import (
|
|||||||
MatchParticipant, Tryout, TryoutRegistration, TeamPlayer,
|
MatchParticipant, Tryout, TryoutRegistration, TeamPlayer,
|
||||||
)
|
)
|
||||||
from werkzeug.utils import secure_filename
|
from werkzeug.utils import secure_filename
|
||||||
from datetime import datetime, timedelta, date as date_type
|
from datetime import datetime, timedelta
|
||||||
from marshmallow import ValidationError
|
from marshmallow import ValidationError
|
||||||
from app.validators import (
|
from app.validators import (
|
||||||
CreateUserSchema, EditUserSchema, EditProfileSchema,
|
CreateUserSchema, EditUserSchema, EditProfileSchema,
|
||||||
UploadContractSchema, OneOnOneRequestSchema,
|
UploadContractSchema,
|
||||||
)
|
)
|
||||||
import requests
|
import requests
|
||||||
|
|
||||||
@@ -31,6 +31,38 @@ ALLOWED_SIGNED_EXTENSIONS = {'pdf'}
|
|||||||
|
|
||||||
users_bp = Blueprint('users', __name__, url_prefix='/users')
|
users_bp = Blueprint('users', __name__, url_prefix='/users')
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Form helpers shared by the schema-validated routes
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def _flash_validation_errors(err):
|
||||||
|
"""Surface marshmallow errors the same way auth.py already does."""
|
||||||
|
for field, messages in err.messages.items():
|
||||||
|
for msg in messages:
|
||||||
|
flash(f'{field}: {msg}', 'danger')
|
||||||
|
|
||||||
|
|
||||||
|
def _form_payload(*, checkboxes=(), list_fields=('games',), optional_blank=('password',)):
|
||||||
|
"""Turn the multi-valued request form into a plain dict for marshmallow.
|
||||||
|
|
||||||
|
request.form.to_dict() keeps only the first value of a repeated key, so
|
||||||
|
list fields have to be re-read with getlist(). Unchecked HTML checkboxes
|
||||||
|
are simply absent from the submission, which is not the same as a schema
|
||||||
|
default, so they are injected explicitly. Blank optional fields are
|
||||||
|
dropped rather than sent as '' — an empty password means "leave the
|
||||||
|
current one alone", not "set the password to the empty string".
|
||||||
|
"""
|
||||||
|
payload = request.form.to_dict()
|
||||||
|
for name in list_fields:
|
||||||
|
payload[name] = request.form.getlist(name)
|
||||||
|
for name in checkboxes:
|
||||||
|
payload[name] = name in request.form
|
||||||
|
for name in optional_blank:
|
||||||
|
if not payload.get(name):
|
||||||
|
payload.pop(name, None)
|
||||||
|
return payload
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Gamertag helper (shared)
|
# Gamertag helper (shared)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -96,21 +128,37 @@ def edit_user(user_id):
|
|||||||
user = User.query.get_or_404(user_id)
|
user = User.query.get_or_404(user_id)
|
||||||
|
|
||||||
if request.method == 'POST':
|
if request.method == 'POST':
|
||||||
full_name = request.form.get('full_name')
|
def _rerender():
|
||||||
email = request.form.get('email')
|
|
||||||
phone = request.form.get('phone')
|
|
||||||
role = request.form.get('role')
|
|
||||||
is_active = request.form.get('is_active_account') == 'on'
|
|
||||||
|
|
||||||
if role not in USER_TYPES:
|
|
||||||
flash('Invalid role selected.', 'danger')
|
|
||||||
return render_template('pages/edit_user.html', user=user, roles=USER_TYPES,
|
return render_template('pages/edit_user.html', user=user, roles=USER_TYPES,
|
||||||
esport_games=ESPORT_GAMES, game_platforms=GAME_PLATFORMS)
|
esport_games=ESPORT_GAMES, game_platforms=GAME_PLATFORMS,
|
||||||
|
user_gamertags={gt.game: {'gamertag': gt.gamertag,
|
||||||
|
'platform': gt.platform}
|
||||||
|
for gt in user.gamertags})
|
||||||
|
|
||||||
selected_games = request.form.getlist('games')
|
try:
|
||||||
discord_username = request.form.get('discord_username', '').strip()
|
validated = EditUserSchema().load(
|
||||||
discord_user_id = request.form.get('discord_user_id', '').strip()
|
_form_payload(checkboxes=('is_active_account',))
|
||||||
league_os_profile = request.form.get('league_os_profile', '').strip()
|
)
|
||||||
|
except ValidationError as err:
|
||||||
|
_flash_validation_errors(err)
|
||||||
|
return _rerender()
|
||||||
|
|
||||||
|
full_name = validated['full_name']
|
||||||
|
email = validated['email']
|
||||||
|
phone = validated.get('phone')
|
||||||
|
role = validated['role']
|
||||||
|
is_active = validated['is_active_account']
|
||||||
|
selected_games = validated.get('games', [])
|
||||||
|
discord_username = validated.get('discord_username')
|
||||||
|
discord_user_id = validated.get('discord_user_id')
|
||||||
|
league_os_profile = validated.get('league_os_profile')
|
||||||
|
|
||||||
|
# Previously absent: the column is unique, so assigning a taken
|
||||||
|
# address surfaced as an IntegrityError, i.e. a 500.
|
||||||
|
clash = User.query.filter(User.email == email, User.id != user.id).first()
|
||||||
|
if clash:
|
||||||
|
flash('Email already in use by another account.', 'danger')
|
||||||
|
return _rerender()
|
||||||
|
|
||||||
# Change role via raw SQL to avoid polymorphic identity corruption.
|
# Change role via raw SQL to avoid polymorphic identity corruption.
|
||||||
# Must discard the entire session because the polymorphic discriminator
|
# Must discard the entire session because the polymorphic discriminator
|
||||||
@@ -137,7 +185,9 @@ def edit_user(user_id):
|
|||||||
|
|
||||||
update_user_gamertags(user, selected_games)
|
update_user_gamertags(user, selected_games)
|
||||||
|
|
||||||
password = request.form.get('password')
|
# Blank means "keep the current password"; anything else has already
|
||||||
|
# been checked against the policy by the schema.
|
||||||
|
password = validated.get('password')
|
||||||
if password:
|
if password:
|
||||||
user.password_hash = hash_password(password)
|
user.password_hash = hash_password(password)
|
||||||
|
|
||||||
@@ -207,17 +257,21 @@ def create_user():
|
|||||||
return redirect(url_for('main.dashboard'))
|
return redirect(url_for('main.dashboard'))
|
||||||
|
|
||||||
if request.method == 'POST':
|
if request.method == 'POST':
|
||||||
username = request.form.get('username')
|
try:
|
||||||
email = request.form.get('email')
|
validated = CreateUserSchema().load(request.form)
|
||||||
password = request.form.get('password')
|
except ValidationError as err:
|
||||||
full_name = request.form.get('full_name')
|
_flash_validation_errors(err)
|
||||||
phone = request.form.get('phone')
|
|
||||||
role = request.form.get('role')
|
|
||||||
|
|
||||||
if role not in USER_TYPES:
|
|
||||||
flash('Invalid role selected.', 'danger')
|
|
||||||
return render_template('pages/create_user.html', roles=USER_TYPES)
|
return render_template('pages/create_user.html', roles=USER_TYPES)
|
||||||
|
|
||||||
|
username = validated['username']
|
||||||
|
email = validated['email']
|
||||||
|
password = validated['password']
|
||||||
|
full_name = validated['full_name']
|
||||||
|
phone = validated.get('phone')
|
||||||
|
# The schema constrains role with OneOf(USER_TYPES), so the former
|
||||||
|
# manual membership check is now redundant.
|
||||||
|
role = validated['role']
|
||||||
|
|
||||||
if User.query.filter_by(username=username).first():
|
if User.query.filter_by(username=username).first():
|
||||||
flash('Username already exists.', 'danger')
|
flash('Username already exists.', 'danger')
|
||||||
return render_template('pages/create_user.html', roles=USER_TYPES)
|
return render_template('pages/create_user.html', roles=USER_TYPES)
|
||||||
@@ -274,15 +328,22 @@ def profile():
|
|||||||
def edit_profile():
|
def edit_profile():
|
||||||
"""Edit the current user's profile."""
|
"""Edit the current user's profile."""
|
||||||
if request.method == 'POST':
|
if request.method == 'POST':
|
||||||
username = request.form.get('username')
|
try:
|
||||||
full_name = request.form.get('full_name')
|
validated = EditProfileSchema().load(_form_payload())
|
||||||
email = request.form.get('email')
|
except ValidationError as err:
|
||||||
phone = request.form.get('phone')
|
_flash_validation_errors(err)
|
||||||
|
return render_template('pages/edit_profile.html', user=current_user,
|
||||||
|
esport_games=ESPORT_GAMES, game_platforms=GAME_PLATFORMS,
|
||||||
|
user_gamertags=current_user.get_gamertags())
|
||||||
|
|
||||||
selected_games = request.form.getlist('games')
|
username = validated['username']
|
||||||
discord_username = request.form.get('discord_username', '').strip()
|
full_name = validated['full_name']
|
||||||
discord_user_id = request.form.get('discord_user_id', '').strip()
|
email = validated['email']
|
||||||
league_os_profile = request.form.get('league_os_profile', '').strip()
|
phone = validated.get('phone')
|
||||||
|
selected_games = validated.get('games', [])
|
||||||
|
discord_username = validated.get('discord_username')
|
||||||
|
discord_user_id = validated.get('discord_user_id')
|
||||||
|
league_os_profile = validated.get('league_os_profile')
|
||||||
|
|
||||||
if username != current_user.username and User.query.filter_by(username=username).first():
|
if username != current_user.username and User.query.filter_by(username=username).first():
|
||||||
flash('Username already taken.', 'danger')
|
flash('Username already taken.', 'danger')
|
||||||
@@ -307,7 +368,9 @@ def edit_profile():
|
|||||||
|
|
||||||
update_user_gamertags(current_user, selected_games)
|
update_user_gamertags(current_user, selected_games)
|
||||||
|
|
||||||
password = request.form.get('password')
|
# Blank means "keep the current password"; anything else has already
|
||||||
|
# been checked against the policy by the schema.
|
||||||
|
password = validated.get('password')
|
||||||
if password:
|
if password:
|
||||||
current_user.password_hash = hash_password(password)
|
current_user.password_hash = hash_password(password)
|
||||||
|
|
||||||
@@ -1043,7 +1106,6 @@ def notes_dashboard():
|
|||||||
).order_by(OneOnOneRequest.created_at.desc()).all()
|
).order_by(OneOnOneRequest.created_at.desc()).all()
|
||||||
|
|
||||||
# For context selectors in the form
|
# For context selectors in the form
|
||||||
from app.models import Team as MatchTeam
|
|
||||||
matches = Match.query.filter(
|
matches = Match.query.filter(
|
||||||
db.or_(Match.created_by == current_user.id, Match.status == 'scheduled'),
|
db.or_(Match.created_by == current_user.id, Match.status == 'scheduled'),
|
||||||
).order_by(Match.date.desc()).limit(20).all()
|
).order_by(Match.date.desc()).limit(20).all()
|
||||||
|
|||||||
Reference in New Issue
Block a user