QUA-002, seconde moitie. Le depot etant formate, l elargissement porte sur
des defauts et non sur du brassage.
Ajoute a la selection : B (bugbear), C4, RET, SIM, UP. Le lot entier n a
produit que 24 signalements sur 76 fichiers -- le code etait plus propre
que l audit ne le craignait. Neuf corriges automatiquement, quinze a la
main.
SIM108 est ignore : forcer un ternaire se lit moins bien que le if/else
qu il remplace, au seul endroit ou il se declenche.
isort (I) n est PAS active. Il reordonnerait les imports de 48 fichiers,
soit une seconde passe de pur brassage juste apres le commit de formatage.
A faire, mais seul.
Deux vrais defauts trouves par les nouvelles regles
- team_matches.edit_match faisait `except ValueError: pass` sur l heure de
debut et l heure de fin, trois lignes sous un champ date qui, lui,
signale et redirige. Une heure mal saisie etait donc acceptee par le
formulaire, jetee, l ancienne valeur conservee -- et la page annoncait
la reussite. Meme traitement que la date desormais.
- backup.py levait BackupError depuis deux blocs `except` sans `from`,
ce qui perdait la cause d origine dans la trace.
Ainsi que : un `return` explicite dans force_https, `%`-formatage remplace
dans log_auth_event (operations de chaine avant journalisation, pas des
gabarits de logger -- la redaction n est pas affectee), une compréhension
inutile, un `set(...)` en compréhension d ensemble, `open(..., 'r')`, une
variable de boucle inutilisee, et `contextlib.suppress` dans conftest.
CI : `ruff format --check` remplace le commentaire qui expliquait pourquoi
il etait absent.
263 tests passent. Les deux nouveaux messages sont traduits ; attention,
pybabel les avait apparies en `fuzzy` avec des entrees « date » existantes,
et une entree fuzzy est ignoree a l execution.
Co-Authored-By: Claude Opus 5 <[email protected]>
QUA-002, premiere moitie. **Ce commit ne fait que reformater** : aucun
changement de comportement, aucune ligne de logique touchee. 72 fichiers,
4 restaient deja conformes. Il est isole exprès, pour que `git log -p` sur
les commits voisins reste lisible.
`quote-style = "preserve"` etait deja pose dans pyproject.toml, ce qui
evite le brassage guillemets simples / doubles : le diff porte sur les
retours a la ligne, l indentation des appels longs et les virgules
finales, pas sur le style de chaine.
Verification : 263 tests passent avant et apres, ruff check propre.
L activation en CI arrive dans le commit suivant, separement, pour que ce
diff-ci ne contienne rien d autre.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-007.
Trois points d entree, trois configurations differentes
python app/app.py 0.0.0.0:10000, debogueur desactive par defaut
python run.py 127.0.0.2:5000, debogueur ACTIVE par defaut
python wsgi.py Waitress, production
Le bloc __main__ de app/app.py disparait : ce module expose la fabrique.
run.py reste le point d entree de developpement, wsgi.py celui de
production, et c est tout.
run.py passait FLASK_DEBUG a 'true' par defaut. Le debogueur Werkzeug
execute du code soumis depuis le navigateur ; un processus lance ainsi et
laisse joignable est un shell distant. Le defaut passe a 'false', avec
l avertissement reecrit pour dire ce que le mode implique reellement.
L hote devient 127.0.0.1 -- 127.0.0.2 est une boucle locale valide mais
inhabituelle -- et hote comme port sont surchargeables par DEV_HOST et
DEV_PORT.
Code mort retire
- discord_bot.py, notify_player_about_one_on_one : jamais appelee, seule
la variante _direct l est.
- evaluations.py, branche else de list_evaluations : elle listait les
evaluations recues, une vue de joueur, alors que les joueurs sont
rediriges au debut de la fonction et que can_evaluate() est vrai pour
les quatre roles restants. Inatteignable.
Les autres elements du constat sont deja resorbes : get_auth_logger est
appelee depuis log_auth_event (OBS-001), et ALLOWED_CONTRACT_EXTENSIONS /
ALLOWED_SIGNED_EXTENSIONS sont lues par pdf_upload_error (SEC-021).
wsgi.py n est pas touche : trusted_proxy et HOST attendent la reponse du
developpeur sur la topologie reelle (nginx sur la meme machine ou non).
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-021 -- rien ne validait le contrat signe
upload_signed_contract se contentait d un nom de fichier non vide.
ALLOWED_SIGNED_EXTENSIONS etait declaree juste a cote et jamais lue. Le
fichier atterrissait sur le disque sous un nom que download_signed_contract
sert ensuite : ce qu un joueur televerse est ce qu un gerant ouvre.
Le nom seul ne suffisait pas non plus cote upload_contract, qui verifiait
`.pdf` en fin de chaine -- payload.pdf ne dit rien des octets.
pdf_upload_error() couvre les deux routes : extension dans la liste, puis
signature %PDF- en tete de flux. Le flux est rembobine, l appelant
enregistre toujours le fichier entier.
OPS-014 -- intent Discord privilegie inutile
Le bot demandait GUILD_MEMBERS et ne s en servait pas : rien n enumere ni
ne recherche de membre de serveur, les personnes sont jointes par le
discord_user_id enregistre sur leur compte. Retire.
message_content reste : on_raw_reaction_add lit le texte de la reponse d un
coach pour consigner un motif de refus.
CI-003 et CI-005 sont deja appliques (permissions: contents: read,
checkout@v4, exclusions de deploiement). L epinglage par SHA des actions
n est pas fait : ce sont des actions GitHub de premiere partie, et
l epingler sans Dependabot echange une exposition contre une autre.
11 tests, dont deux verifient que signer un contrat marche toujours et
qu un autre joueur ne peut pas le faire.
Co-Authored-By: Claude Opus 5 <[email protected]>
Quatre constats de la liste des gains rapides, tous sur auth.py.
SEC-017 -- enumeration de comptes
Le formulaire repondait "il vous reste 3 tentative(s)" a un compte connu et
"verifiez le nom d utilisateur et le mot de passe" a un inconnu. Le decompte
lui-meme etait la fuite : la meme information, etalee sur cinq requetes.
S y ajoutait un ecart de temps de reponse, check_password n etant appele que
si la ligne existait -- scrypt est cher, l ecart est mesurable.
Un seul message pour tous les echecs, et la verification s execute
desormais sur les deux branches : contre un hachage aleatoire tire une fois
par processus quand l identifiant n existe pas.
SEC-018 -- verrou de compte declenchable par un tiers
Cinq mauvaises reponses mettaient un compte connu hors service pendant
quinze minutes, indefiniment renouvelables. Sur un compte president, c est
toute l administration, et aucun ecran ne permettait de defaire.
Le compteur et la fenetre restent -- ce sont la trace qu un administrateur
lit quand un compte est pilonne, et la fenetre double jusqu a un plafond.
Ce qui change : de bons identifiants passent, fenetre ouverte ou non, et
remettent le compteur a zero. Le proprietaire du compte ne peut plus etre
bloque par un tiers.
Ce que cela coute, dit franchement : un verrou dur n arretait de toute
facon pas un attaquant ayant trouve le mot de passe -- il lui suffisait
d attendre. Le debit de tentatives reste borne par la limite de 10/minute
par IP. Une limite par couple (compte, IP) demanderait un stockage dedie ;
elle attend Alembic.
L evenement account.locked devient account.throttled : "locked" affirmait
plus que ce qui se passe.
SEC-019 -- deconnexion en GET
/auth/logout n avait pas de methods, donc GET, donc hors protection CSRF :
n importe quelle page pouvait deconnecter un visiteur avec une balise img.
La route passe en POST et l entree de navigation devient un formulaire avec
jeton. Le style suit -- les regles .nav-links visaient les liens seuls.
SEC-020 -- validation de redirection
is_safe_url interrogeait urlparse().netloc. urlparse lit /\evil.com comme
un chemin, sans netloc ; plusieurs navigateurs normalisent l antislash en
barre oblique avant de resoudre, ce qui en fait //evil.com. La fonction
refuse maintenant antislash et caracteres de controle, exige un chemin
enracine, et compare l origine explicitement.
Le xfail(strict) qui documentait SEC-017 est leve. 40 tests dans
test_auth_session.py, dont la table des cibles refusees.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-008. `role` est le discriminateur polymorphe, et SQLAlchemy fixe la
classe d une instance au chargement. edit_user ecrivait donc la colonne par
un UPDATE de niveau instruction puis relisait la ligne -- c est correct.
La suite ne l etait pas.
db.session.commit()
db.session.remove() # discard stale session entirely
user = User.query.get(user_id_local)
Deux defauts dans ces trois lignes.
1. remove() jette la session entiere. Tout ce que la requete tenait encore
se retrouvait detache, current_user compris ; le moindre acces a un
attribut ensuite levait DetachedInstanceError. La route ne survivait
qu en ayant recopie le nom et l id de l acteur dans des variables
locales avant -- un contournement, pas la correction. Un expunge de la
seule instance perimee suffit.
2. Le commit intermediaire coupait l edition en deux. Le role etait acquis
avant que le reste du formulaire soit applique : une erreur ensuite
laissait un compte promu et le reste perdu, sans qu aucune interface ne
le signale. Sans ce commit, l UPDATE reste dans la transaction, la
relecture le voit, et l ensemble part en un seul commit.
Les evenements d audit passent apres le commit. account.role_changed etait
journalise avant l UPDATE : le journal affirmait un changement que la
transaction pouvait encore annuler.
tests/test_role_change.py, 6 tests. Un seul echoue sur le code d avant --
celui de l atomicite ; les cinq autres fixent le comportement qui marchait
deja, pour que la suite de la vague D ne le casse pas.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-002. La question "a quelles equipes ce coach est-il rattache ?" etait
posee a huit endroits, de cinq facons differentes, et trois d entre elles
donnaient une mauvaise reponse en production.
Le motif fautif, present tel quel dans six routes :
OrgTeam.query.filter_by(coach_id=user.id).first()
Il repond au plus une equipe, et seulement par la colonne heritee. Deux
pannes en decoulaient, silencieuses -- les pages s affichaient, vides :
- un coach rattache uniquement par la relation many-to-many n avait
aucune equipe, donc aucun joueur, aucun contrat, aucune note d equipe,
aucun match a venir sur son tableau de bord ;
- un coach de deux equipes n en voyait qu une. Le formulaire de contrat
lui proposait la moitie de son effectif, alors que la route POST
acceptait l autre moitie.
app/permissions.py devient le module ou la question se pose une fois :
coach_org_teams, manager_org_teams, attached_org_teams, visible_org_teams,
can_manage_org_team, org_team_player_ids, coach_player_ids,
can_manage_player_contract, coach_tryouts, coach_manages_tryout. Toutes
lisent les deux rattachements et toutes les equipes.
Le meme ecart existait dans le modele : Coach.get_visible_tryouts ne lisait
que la relation m2m -- calendrier vide pour un coach rattache par la
colonne -- et can_manage_this_tryout ignorait la colonne pour l equipe
cible. Les deux delegent desormais.
Corrections de portee, au passage
- one_on_one lisait org_team.coach_id : un joueur dont l equipe declare
ses coachs par la relation etait informe qu il n avait pas de coach, et
le formulaire de demande restait ferme. Passe par get_coaches(), qui
retombe deja sur la colonne heritee.
- notes_dashboard conditionnait les notes personnelles du coach a
l existence d une equipe : un coach sans equipe ne voyait pas ses
propres notes.
- can_manage_team_match reformulait can_manage_this_org_team ; la
reformulation avait derive. Elle appelle maintenant la regle.
Limite assumee : le panneau de notes d equipe reste ecrit pour une seule
equipe et affiche donc la premiere. La resolution est corrigee, la mise en
page multi-equipes ne l est pas -- c est un choix produit, pas un bug.
14 tests ajoutes. Cinq echouent sur le code d avant, verifie en remettant
les routes et le modele a leur etat precedent.
ARCH-001 fera disparaitre la colonne heritee ; cela demande une migration
de donnees, donc Alembic. D ici la, ce module est ce qui rend la
duplication inoffensive.
Co-Authored-By: Claude Opus 5 <[email protected]>
198 appels flash dans les sept modules de routes, plus les 23 messages de
validation de app/validators.py. Le catalogue compte desormais 631 chaines,
aucune non traduite.
validators.py utilise lazy_gettext : les champs de schema sont construits a
l'import, donc avant qu'une requete existe. Un gettext ordinaire s'y
resoudrait une seule fois, dans la langue active au demarrage.
Un bug introduit par la conversion, puis corrige
Le convertisseur automatique ne voyait que le premier litteral d'un appel
flash, ce qui a casse deux chaines concatenees sur plusieurs lignes dans
users.py -- le resultat n'etait meme pas du Python valide. Ma premiere
verification ne l'a pas vu : elle enchainait py_compile sur head, or head
reussit toujours, donc le "OK" s'affichait quoi qu'il arrive. Les deux
appels sont reecrits et la verification refaite correctement.
Un bug plus interessant, revele par le test de fumee
La langue choisie ne survivait pas a la connexion. login() et logout()
appellent tous deux session.clear() -- l'un contre la fixation de session,
l'autre pour terminer la session -- et le choix de langue partait avec le
reste. Concretement : quelqu'un qui lisait la page de connexion en anglais
se retrouvait en francais des qu'il se connectait.
La langue est une preference d'affichage, pas un etat appartenant au
compte. Les deux endroits la reportent maintenant explicitement, a cote du
jeton CSRF. Quatre tests couvrent le cas, dont un qui verifie que corriger
une cle preservee n'a pas fait tomber l'autre.
Detail de nommage : le convertisseur avait genere %(value)s pour une
expression conditionnelle, ce qui n'aide pas un traducteur. Renomme en
%(player)s.
Les 14 traductions ecrites avec une apostrophe droite sont normalisees en
apostrophe typographique. Sans consequence en HTML, ou ' s'affiche
correctement -- mais les blocs <script> ne decodent pas les entites, et
autant que le catalogue soit homogene.
200 tests.
Co-Authored-By: Claude Opus 5 <[email protected]>
DATA-004, DATA-005, DATA-006. L'audit les classait "forte probabilite" faute
de pouvoir les executer. Les tests les confirment : ce sont des bugs averes,
declenchables par tout manager ou administrateur depuis l'interface.
Erreurs reellement obtenues avant correction :
NOT NULL constraint failed: match_participants.match_id
NOT NULL constraint failed: team_matches.org_team_id
Supprimer un match (DATA-004)
Match.participants n'avait pas de cascade. SQLAlchemy tentait donc de
detacher les participants en mettant match_id a NULL, ce que la colonne
refuse. Tout match ayant eu des participants etait indestructible.
TeamMatch.participants declarait deja delete-orphan ; Match non.
Supprimer un tryout (DATA-006)
Les PersonalNote pointant vers ses matchs, equipes ou vers lui-meme
n'etaient pas traitees.
Supprimer une equipe (DATA-005)
TeamNote.org_team_id et TeamMatch.org_team_id sont NOT NULL et n'etaient
pas traites du tout. De plus la fonction validait trois fois : un echec au
troisieme temps laissait les tryouts detaches et les joueurs retires sans
que l'equipe soit supprimee -- un etat incoherent que rien ne rattrapait.
Une seule transaction desormais.
Regle appliquee, uniforme
Ce qui n'a de sens que dans le parent est supprime avec lui : participants,
membres, notes d'equipe, matchs de saison.
Ce qui lui survit est seulement detache : les notes personnelles sont les
observations d'un coach sur un joueur, pas des donnees de tryout. Les
supprimer avec le tryout detruirait du contenu sans rapport. Idem pour les
contrats et les demandes de rencontre individuelle.
Fidelite des tests
conftest.py active PRAGMA foreign_keys=ON. SQLite ignore les cles
etrangeres par defaut ; PostgreSQL les applique toujours. Sans ce reglage,
la suite pouvait valider une suppression qui echoue en production --
precisement la classe de bug corrigee ici. Les 146 tests passent avec les
contraintes actives.
9 tests, dont trois qui verifient que les entites survivantes survivent
vraiment : une note garde son contenu et perd son contexte, un tryout
survit a l'equipe qu'il visait.
Co-Authored-By: Claude Opus 5 <[email protected]>
Le site s'affiche desormais en francais par defaut, avec un selecteur de
langue permettant de basculer vers l'anglais.
Choix de conception : les chaines sources restent en anglais
Elles servent d'identifiants gettext, et le francais est fourni par
catalogue avec BABEL_DEFAULT_LOCALE = 'fr'. Le code reste ainsi dans une
seule langue -- la meme que ses commentaires et docstrings -- tandis que
ce qu'un membre voit est du francais.
Consequence qui rend la migration praticable : une chaine non encore
traduite retombe en anglais, pas sur un identifiant brut. Les gabarits
peuvent donc etre migres un par un sans jamais laisser le site a moitie
casse.
Selection de la langue (app/i18n.py)
1. choix explicite via le selecteur, garde en session
2. sinon en-tete Accept-Language du navigateur, restreint a fr et en
3. sinon francais
Un choix explicite prime toujours, y compris sur un navigateur anglophone.
Selecteur
Extrait en partiel et inclus dans les deux branches de la mise en page :
barre laterale une fois connecte, ET page d'authentification. Quelqu'un
qui ne lit pas la langue courante doit pouvoir en changer AVANT de se
connecter -- le laisser derriere l'authentification aurait ete un defaut
d'accessibilite. Chaque langue est ecrite dans sa propre langue.
La route /lang/<locale> valide le Referer avant de rediriger : sans ce
controle, elle constituait une redirection ouverte.
Migre dans cette passe
navigation complete, page de connexion, les cinq pages d'erreur, et
l'integralite des messages flash de routes/auth.py. 64 chaines, dont
aucune non traduite.
Verification
25 tests, dont deux garde-fous d'integrite : un catalogue .mo manquant
ou une entree non traduite font echouer la suite. Sans cela, une
compilation oubliee servirait de l'anglais partout, en silence et sans
rien dans les journaux.
Un test existant a du etre corrige, et c'est instructif
test_login_failure_message_does_not_reveal_account_existence cherchait la
sous-chaine anglaise 'attempt(s) remaining'. La page etant desormais en
francais, elle etait absente des deux cotes, l'assertion passait, et le
mode strict a signale le faux succes. Le test comparait donc l'anglais,
pas le comportement. Il compare desormais les messages flash rendus,
quelle que soit la langue. La faille SEC-AUTH-006 reste ouverte, et le
test la documente toujours.
Les catalogues .po ET .mo sont versionnes : le deploiement est un simple
miroir de fichiers, sans etape de compilation. messages.pot, regenerable,
ne l'est pas.
docs/translations.md documente le processus, les deux pieges (concatenation
de phrases, traduction a l'import), et l'etat de la migration. A noter pour
la suite : les chaines dans les blocs <script> ne peuvent pas etre balisees
telles quelles, il faudra les passer par des attributs data- -- ce qui
rejoint le chantier de sortie de unsafe-inline (OPS-010).
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-004 et SEC-AUTHZ-005. La meme question -- ce coach peut-il agir
sur ce joueur ? -- recevait cinq reponses differentes selon la route :
teams.py:add_player_note verifiait l'appartenance via TeamPlayer
users.py, 4 routes de notes ne verifiaient rien au-dela d'isinstance
contract.py:can_view interrogeait la colonne heritee coach_id, et
traitait un team_id nul comme un joker
Consequences levees
- tout coach pouvait ecrire une note nominative sur tout joueur du club.
Ces notes sont visibles par le joueur concerne.
- tout coach figurant dans OrgTeam.coach_id pouvait lire n'importe quel
contrat sans equipe rattachee. Or upload_contract laisse team_id nul des
que le joueur n'appartient a aucune equipe : la condition
`not self.team_id or ...` ouvrait donc largement.
- symetriquement, un coach rattache uniquement par la relation
many-to-many ne voyait aucun contrat.
app/permissions.py
Premier pas concret vers ARCH-002, sans refonte : un module unique, pas
une couche de services. coach_org_team_ids() lit la relation m2m ET la
colonne heritee, donc le deuxieme coach d'une equipe cesse d'etre
invisible. coach_can_access_player() accorde l'acces si le joueur est sur
une equipe du coach, ou inscrit a un tryout qu'il gere, ou participant a
un match de ce tryout.
14 tests, dont deux verifient que les chemins legitimes fonctionnent
toujours : un coach note bien son propre joueur, et voit bien son contrat.
Note : la regle metier retenue -- equipe OU tryout -- est une lecture du
comportement existant, pas une decision produit. Si le club attend autre
chose, c'est desormais un seul endroit a changer.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-007 - auto-verrouillage de l'administration
Le changement de role n'excluait ni l'utilisateur courant, ni le dernier
compte admin actif. Une seule manipulation suffisait a transformer le seul
president en joueur, et plus aucune interface ne permettait de revenir en
arriere : il fallait une intervention directe en base.
Deux gardes distinctes, parce que ce sont deux erreurs differentes :
- changer son propre role est refuse, meme s'il reste d'autres admins.
Un president qui veut se retrograder doit le faire faire par un autre.
- retrograder le dernier admin actif est refuse.
Le decompte exclut les comptes desactives : trois admins dont deux
desactives, cela fait un seul administrateur reel.
SEC-WEB-003 - CORS ouvert par defaut
Sans CORS_ALLOWED_ORIGINS, la branche else appelait
CORS(app, supports_credentials=True) sans argument origins. flask-cors
retient alors '*' et, les identifiants etant autorises, renvoie en echo
l'Origin de l'appelant avec Access-Control-Allow-Credentials: true --
l'inverse exact de ce qu'annonçait le commentaire.
L'exploitation etait bloquee par SESSION_COOKIE_SAMESITE = 'Lax', qui
empeche le navigateur de joindre le cookie de session a une requete
fetch inter-site. Toute la protection tenait donc a ce seul reglage.
Cette application rend du HTML en meme origine : elle n'a besoin
d'aucune politique CORS. La branche par defaut est supprimee, la
configuration explicite reste possible.
5 tests ajoutes, dont deux verifient que les chemins legitimes continuent
de fonctionner : un autre administrateur reste retrogradable, et une
origine explicitement configuree est toujours honoree.
Co-Authored-By: Claude Opus 5 <[email protected]>
OBS-001. logging_config.py configurait un fichier auth.log avec rotation,
un logger nomme 'team_tryouts.auth' et un filtre de redaction. Mais
get_auth_logger n'etait importe nulle part : le fichier etait cree et
restait vide. Aucune connexion, aucun echec, aucun verrouillage, aucun
changement de role, aucune suppression de compte ne laissait de trace.
En cas de suspicion de compromission, il n'y avait rien a consulter.
Ajout de log_auth_event(event, **fields), qui emet des paires cle=valeur
ordonnees -- greppable sans dependance de journalisation JSON.
Evenements couverts
authentification login.success, login.failure,
login.failure.unknown_user, login.rejected.locked,
login.rejected.deactivated, account.locked, logout,
account.registered
administration account.created_by_admin, account.updated,
account.role_changed (avec ancien et nouveau role),
account.deleted, account.password_reset_by_admin
libre-service account.password_changed
Le champ ip vient de request.remote_addr, donc de X-Forwarded-For. Tant que
Waitress tourne avec trusted_proxy='*' (SEC-WEB-002), cette valeur est
choisie par l'appelant : c'est une indication, pas une preuve. Le point est
documente dans la docstring.
Un test a fait remonter ARCH-003, jusqu'ici classe comme fragilite latente
Journaliser en fin de edit_user levait DetachedInstanceError : le
changement de role appelle db.session.remove() en plein cycle de requete,
ce qui detache current_user de la session. Le code s'en tirait parce qu'il
redirigeait immediatement sans plus y toucher. L'identite de l'acteur est
desormais capturee en debut de traitement. Le constat est donc confirme
comme reel, et non plus seulement probable -- sa correction de fond reste
au programme.
15 tests, dont 4 sur le filtre de redaction lui-meme : c'est un controle de
securite, il doit etre verifie.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTH-004. L'URL d'autorisation ne portait que client_id, redirect_uri,
response_type et scope. Sans state, le callback acceptait n'importe quel
code d'autorisation qu'on lui presentait.
Scenario ferme : un attaquant obtient un code pour SON compte Discord, puis
fait charger l'URL de callback par le navigateur de la victime. Le
formulaire d'inscription de la victime se retrouve pre-rempli avec
l'identite Discord de l'attaquant. C'est le login CSRF decrit par la
RFC 6749 §10.12.
Mise en oeuvre
secrets.token_urlsafe(32) genere le jeton, stocke en session avant la
redirection. Le callback le compare en temps constant avec
secrets.compare_digest, et le consomme systematiquement -- valide ou non --
pour qu'il ne puisse pas etre rejoue. Le controle intervient avant
l'echange du code : un callback rejete ne declenche aucun appel reseau.
Deux corrections accessoires sur le meme chemin
- DISCORD_REDIRECT_URI est desormais verifie au meme titre que
DISCORD_CLIENT_ID. Non defini, il faisait lever requests.utils.quote(None)
au lieu de signaler un probleme de configuration.
- la construction de la chaine de requete passe a urlencode() plutot qu'a
une concatenation manuelle.
9 tests : presence du state, stockage en session, unicite entre deux
demandes, rejet sans state, avec un state forge, sans demande prealable,
et non-rejouabilite.
Reste ouvert : l'identite Discord obtenue reste ensuite reinjectee par un
champ cache du formulaire (SEC-AUTH-005). Le state protege la liaison, pas
encore la valeur elle-meme.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-XSS-001. Chaine complete : un nom d'utilisateur libre arrivait dans le
DOM d'un coach ou d'un administrateur, en meme origine, avec sa session.
La CSP autorisant 'unsafe-inline', rien ne l'arretait.
Cote serveur - la cause
/matches/api/events construisait de la presentation dans un champ JSON :
match_desc = participants_str + f"<br>{match.description}"
Le navigateur deposait cette valeur telle quelle dans innerHTML. Les noms
de joueurs y transitaient sans echappement -- et il ne pouvait pas y en
avoir : c'est du JSON, pas du HTML.
Les deux valeurs etaient deja des cles distinctes du payload. La
concatenation faisait donc aussi afficher les participants deux fois dans
le modal : une fois dans "Teams", une fois en tete de "Description".
Corriger la faille corrige l'affichage.
Cote navigateur - le sink
showEventModal assemblait une chaine HTML puis l'affectait a innerHTML.
Remplace par une construction de noeuds : makeEl / detailItem /
multilineNode / teamNode passent tout texte par textContent. Les retours
a la ligne d'une description restent rendus, via des <br> crees en dur.
Les deux listes deroulantes concatenaient egalement titres de tryout et
noms d'equipe dans innerHTML. Remplacees par new Option(), dont le
premier argument est pose en texte.
Verification
5 tests sur le contrat de l'API, dont un avec un nom d'utilisateur
hostile ecrit directement en base -- ce que la validation refuse
desormais, mais que des lignes anterieures peuvent contenir.
Le JavaScript inline extrait passe `node --check`.
Reste ouvert : la CSP autorise toujours 'unsafe-inline' (SEC-WEB-001), donc
la defense en profondeur manque encore. Suivi en OPS-010.
Co-Authored-By: Claude Opus 5 <[email protected]>
Aucun changement de comportement. Les chaines concernees ne contenaient
aucune substitution.
A noter pour plus tard : run_https.py conserve une banniere en caracteres
semi-graphiques, du meme type que celle qui faisait planter security_scan.py
sur une console Windows en cp1252. Le script n'etant lance qu'en
developpement et de facon explicite, le point est signale sans etre corrige
ici.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-002. add_to_team recevait tryout_id et team_id independamment
dans l'URL, controlait l'autorisation sur le tryout, puis operait sur
l'equipe sans jamais etablir de lien entre les deux. Un gestionnaire du
tryout A pouvait donc modifier une equipe du tryout B.
Le lint pointait exactement dessus : `team` etait charge ligne 463 puis
jamais utilise. La correction automatique proposee etait de supprimer la
variable, ce qui aurait fait taire l'avertissement en cimentant la faille.
Elle est desormais utilisee pour ce a quoi elle servait.
Trois defauts sur la meme route, corriges ensemble :
- team.tryout_id != tryout_id repond maintenant 404
- seuls les joueurs inscrits au tryout peuvent rejoindre ses equipes
- int(player_id) sur une entree de formulaire brute levait ValueError,
donc une erreur 500, sur toute valeur non numerique
Nettoyage automatique par ruff : 34 imports et variables morts retires
sur l'ensemble du paquet. La suite de tests a servi de filet, elle passe
a l'identique avant et apres. Aucun changement de comportement.
A noter, OneOnOneRequestSchema figurait aussi parmi les imports morts :
c'est un quatrieme schema jamais appele, la route one_on_one validant ses
dates a la main. Unifier la validation reste a faire (ARCH-005).
Co-Authored-By: Claude Opus 5 <[email protected]>
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]>
Deux protections etaient configurees sans avoir d'effet.
Expiration de session
app.py:73 definit PERMANENT_SESSION_LIFETIME = 3600, mais Flask
n'applique cette duree qu'aux sessions marquees permanentes. Aucune
occurrence de session.permanent n'existait dans app/. Le cookie emis
etait donc un cookie de session navigateur, sans expiration, et le
serveur ne verifiait aucune anciennete. Ajout de session.permanent
juste avant login_user, apres la rotation anti-fixation.
Desactivation de compte
is_active_account n'etait consulte qu'au moment du login (auth.py:141).
User n'ayant pas surcharge is_active, UserMixin renvoyait True en
permanence. Desactiver un compte empechait donc la reconnexion mais
laissait vivre la session en cours.
La propriete is_active seule ne suffit pas : Flask-Login ne la consulte
qu'a l'appel de login_user, jamais lors de la restauration d'une session
depuis le cookie. Le verrou effectif est donc dans load_user, qui renvoie
desormais None pour un compte desactive. La propriete est ajoutee malgre
tout pour que login_user soit coherent avec le chargeur.
load_user passe au passage de Query.get() (API heritee, avertie en
SQLAlchemy 2.0) a db.session.get(), et tolere un identifiant non entier
sans lever.
Co-Authored-By: Claude Opus 5 <[email protected]>
