From a793b7ed0d6cd94c15fd81587ca8374bc3261a90 Mon Sep 17 00:00:00 2001 From: GGThed Date: Fri, 7 Aug 2026 19:47:00 -0400 Subject: [PATCH] fix(authz): verifier le rattachement equipe/tryout, et nettoyer le code mort 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 --- app/discord_bot.py | 7 +++---- app/models/user_model/coach.py | 1 - app/routes/evaluations.py | 4 ++-- app/routes/main.py | 4 ++-- app/routes/matches.py | 9 ++++----- app/routes/team_matches.py | 2 +- app/routes/teams.py | 3 +-- app/routes/tryouts.py | 23 ++++++++++++++++++++--- 8 files changed, 33 insertions(+), 20 deletions(-) diff --git a/app/discord_bot.py b/app/discord_bot.py index c1bf3c8..24e74b9 100644 --- a/app/discord_bot.py +++ b/app/discord_bot.py @@ -15,7 +15,7 @@ import traceback from datetime import datetime, timedelta from zoneinfo import ZoneInfo from queue import Queue, Empty -from discord import Forbidden, HTTPException, NotFound, Intents +from discord import Intents from discord.ext import commands from apscheduler.schedulers.asyncio import AsyncIOScheduler from apscheduler.triggers.cron import CronTrigger @@ -409,7 +409,7 @@ class TeamTryoutsBot(commands.Bot): async def handle_attendance_confirm(self, player, message_id, reference_id, channel): """Handle player confirming attendance for a match/tryout.""" try: - from app.models import MatchParticipant, TryoutRegistration, Match, Tryout + from app.models import MatchParticipant, TryoutRegistration from app.extensions import db request_info = self.pending_requests[message_id] @@ -438,7 +438,7 @@ class TeamTryoutsBot(commands.Bot): async def handle_attendance_decline(self, player, message_id, reference_id, channel): """Handle player declining attendance for a match/tryout.""" try: - from app.models import MatchParticipant, TryoutRegistration, Match, Tryout + from app.models import MatchParticipant, TryoutRegistration from app.extensions import db request_info = self.pending_requests[message_id] @@ -564,7 +564,6 @@ class TeamTryoutsBot(commands.Bot): """Internal implementation of daily reminders with proper app context.""" try: from app.models import Match, Tryout, MatchParticipant, TryoutRegistration, OneOnOneRequest - from app.extensions import db from sqlalchemy.orm import joinedload now = datetime.now(self.timezone) diff --git a/app/models/user_model/coach.py b/app/models/user_model/coach.py index d05a865..57c129d 100644 --- a/app/models/user_model/coach.py +++ b/app/models/user_model/coach.py @@ -42,7 +42,6 @@ class Coach(User): def get_visible_tryouts(self): from app.models.tryout.tryout import Tryout - from app.models._associations import tryout_coaches team_ids = [t.id for t in self.coached_org_teams.all()] conditions = [] if team_ids: diff --git a/app/routes/evaluations.py b/app/routes/evaluations.py index 6efa290..31adfdf 100644 --- a/app/routes/evaluations.py +++ b/app/routes/evaluations.py @@ -7,9 +7,9 @@ from flask import Blueprint, render_template, redirect, url_for, flash, request from flask_login import login_required, current_user from app.extensions import db from app.models import ( - Admin, Coach, Manager, Player, + Admin, Player, User, Tryout, Evaluation, TryoutRegistration, - OrgTeam, GAME_POSITIONS, + GAME_POSITIONS, ) from sqlalchemy import func from sqlalchemy.orm import aliased diff --git a/app/routes/main.py b/app/routes/main.py index 1927546..d69d85c 100644 --- a/app/routes/main.py +++ b/app/routes/main.py @@ -3,12 +3,12 @@ Uses polymorphic isinstance checks instead of role-string comparisons. """ -from flask import Blueprint, render_template, redirect, url_for, flash +from flask import Blueprint, render_template, redirect, url_for from flask_login import login_required, current_user from app.extensions import db from app.models import ( Admin, Manager, Coach, Player, Scout, - User, Tryout, Evaluation, TryoutRegistration, Team, TeamMember, + User, Tryout, Evaluation, TryoutRegistration, TeamMember, Match, MatchParticipant, OrgTeam, ) from sqlalchemy import func diff --git a/app/routes/matches.py b/app/routes/matches.py index 457cacb..a9dc0cf 100644 --- a/app/routes/matches.py +++ b/app/routes/matches.py @@ -9,10 +9,10 @@ from app.extensions import db from app.models import ( Admin, Manager, Coach, Player, Scout, User, Tryout, Match, MatchParticipant, Team, TeamMember, - OrgTeam, TryoutRegistration, PlayerDisponibility, + TryoutRegistration, PlayerDisponibility, OneOnOneRequest, ) -from datetime import datetime, time, timedelta +from datetime import datetime, timedelta from app.discord_bot import send_schedule_notification matches_bp = Blueprint('matches', __name__, url_prefix='/matches') @@ -513,13 +513,12 @@ def delete_match(match_id): def get_players_available_at_time(date_str, time_str): """Get list of player IDs available at a specific date and time.""" try: - date_obj = datetime.strptime(date_str, '%Y-%m-%d').date() + parsed_date = datetime.strptime(date_str, '%Y-%m-%d') time_obj = datetime.strptime(time_str, '%H:%M').time() except (ValueError, TypeError): return [] - date_for_day = datetime.strptime(date_str, '%Y-%m-%d') - day_of_week = date_for_day.weekday() + day_of_week = parsed_date.weekday() players = User.query.filter_by(role='player', is_active_account=True).all() available_players = [] diff --git a/app/routes/team_matches.py b/app/routes/team_matches.py index 3d2115f..ef9914c 100644 --- a/app/routes/team_matches.py +++ b/app/routes/team_matches.py @@ -8,7 +8,7 @@ from flask_login import login_required, current_user from app.extensions import db from app.models import ( Admin, Manager, Coach, Player, - OrgTeam, User, TeamMatch, TeamMatchParticipant, TeamPlayer, + OrgTeam, TeamMatch, TeamMatchParticipant, TeamPlayer, ) from datetime import datetime, timedelta from app.discord_bot import send_schedule_notification diff --git a/app/routes/teams.py b/app/routes/teams.py index 6e1b410..93c1ef5 100644 --- a/app/routes/teams.py +++ b/app/routes/teams.py @@ -8,8 +8,7 @@ from flask_login import login_required, current_user from app.extensions import db from app.models import ( Admin, Manager, Coach, Player, - OrgTeam, User, Team, TeamMember, - PersonalNote, TeamNote, Tryout, TeamPlayer, + OrgTeam, User, PersonalNote, TeamNote, Tryout, TeamPlayer, ) from datetime import datetime diff --git a/app/routes/tryouts.py b/app/routes/tryouts.py index df11f97..501c204 100644 --- a/app/routes/tryouts.py +++ b/app/routes/tryouts.py @@ -4,7 +4,7 @@ This module handles CRUD operations for tryouts and player registrations. Uses polymorphic isinstance checks instead of role-string comparisons. """ -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_required, current_user from app.extensions import db from app.models import ( @@ -466,13 +466,30 @@ def add_to_team(tryout_id, team_id): flash('Permission denied.', 'danger') return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) - player_id = request.form.get('player_id') + # The two ids arrive independently in the URL. Without this check, being + # allowed to manage tryout A was enough to modify a team belonging to + # tryout B, since only the tryout was authorised. + if team.tryout_id != tryout_id: + abort(404) + + player_id = request.form.get('player_id', type=int) + if not player_id: + flash('Please select a player.', 'danger') + return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) + + # Only players registered for this tryout may be placed on its teams. + is_registered = TryoutRegistration.query.filter_by( + tryout_id=tryout_id, player_id=player_id).first() is not None + if not is_registered: + flash('That player is not registered for this tryout.', 'danger') + return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id)) + position = request.form.get('position', '') existing = TeamMember.query.filter_by(team_id=team_id, player_id=player_id).first() if existing: flash('Player is already on this team.', 'info') else: - member = TeamMember(team_id=team_id, player_id=int(player_id), position=position) + member = TeamMember(team_id=team_id, player_id=player_id, position=position) db.session.add(member) db.session.commit() flash('Player added to team!', 'success')