Files
team-tryouts/app/routes/teams.py
T
GGThedandClaude Opus 5 20dabecd67 fix(authz): finir SEC-16, que mon propre correctif avait laisse a moitie
La vague K a pose un schema sur create_team et edit_team et a laisse cinq
routes soeurs du meme fichier lire int(request.form.get(...)) : add_coach,
add_manager, remove_coach, remove_manager et add_player. Un identifiant non
numerique y etait un 500 dans chacune.

C'est exactement la lecon que ce projet repete depuis la vague D -- corriger
un motif fautif dans une seule couche le laisse dans les autres -- et cette
fois c'est le correctif lui-meme qui l'a commise. Notee comme telle.

Deux defauts de plus, trouves en finissant.

add_player ecrivait status tel quel dans une colonne NOT NULL String(20). Et
toggle_player_status lit "substitute si status == starter, sinon starter" :
une valeur inconnue devenait donc starter a la premiere bascule, c'est-a-dire
promouvait son porteur. Liste blanche dans TEAM_PLAYER_STATUSES.

Et aucune de ces routes ne regardait is_active_account. La requete qui
alimente la liste deroulante des joueurs ne le filtrait pas non plus, alors
que les deux requetes juste au-dessus, coachs et gerants, le posaient -- deux
lignes d'ecart, meme fichier. Un compte desactive etait donc propose et
accepte, alors que is_active_account est precisement ce qui dit que la
personne a quitte le club. Meme oubli dans tryouts.py.

_staff_member delegue desormais a _assignable au lieu de repeter isinstance :
deux fonctions du meme fichier repondant differemment a "ce compte peut-il
prendre ce role" est la forme de tous les defauts qu'a eus ce module.

Verifie par mutation : retirer le controle d'activite ou la liste blanche
fait tomber trois tests.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-11 20:42:26 -04:00

613 lines
22 KiB
Python

"""Organization team management routes.
Uses polymorphic isinstance checks instead of role-string comparisons.
"""
from datetime import datetime
from flask import Blueprint, flash, jsonify, redirect, render_template, request, url_for
from flask_babel import gettext as _
from flask_login import current_user, login_required
from marshmallow import ValidationError
from app.api import json_endpoint
from app.extensions import db
from app.forms import flash_validation_errors, form_payload
from app.models import (
Admin,
Coach,
Contract,
Manager,
OneOnOneRequest,
OrgTeam,
PersonalNote,
Player,
TeamMatch,
TeamNote,
TeamPlayer,
Tryout,
User,
)
from app.permissions import visible_org_teams
from app.validators import OrgTeamSchema, TeamPlayerSchema, TeamStaffSchema
teams_bp = Blueprint('teams', __name__, url_prefix='/teams')
@teams_bp.route('')
@login_required
def list_teams():
"""List all organization teams visible to the current user."""
can_manage = current_user.can_manage_teams()
if isinstance(current_user, Player):
flash(_('Use My Team(s) to view your teams.'), 'info')
return redirect(url_for('teams.my_teams'))
if not isinstance(current_user, (Admin, Coach, Manager)):
flash(_('You do not have permission to view teams.'), 'danger')
return redirect(url_for('main.dashboard'))
teams = visible_org_teams(current_user)
coaches = (
User.query.filter_by(role='coach', is_active_account=True).order_by(User.username).all()
)
managers = (
User.query.filter_by(role='manager', is_active_account=True).order_by(User.username).all()
)
# is_active_account, like the two queries above it. Without it the "add
# player" select offered accounts that had been deactivated, and
# add_player accepted them.
all_players = (
User.query.filter_by(role='player', is_active_account=True).order_by(User.username).all()
)
return render_template(
'pages/teams.html',
teams=teams,
coaches=coaches,
managers=managers,
all_players=all_players,
can_manage=can_manage,
)
@teams_bp.route('/my-teams')
@login_required
def my_teams():
"""View the player's own teams with upcoming matches."""
if not isinstance(current_user, Player):
flash(_('This page is for players.'), 'info')
return redirect(url_for('teams.list_teams'))
from app.models import TeamMatch, TeamMatchParticipant
player_teams = current_user.get_org_teams()
now = datetime.utcnow()
team_data = []
for org_team in player_teams:
matches = (
TeamMatch.query.filter(
TeamMatch.org_team_id == org_team.id,
TeamMatch.status == 'scheduled',
)
.order_by(TeamMatch.date.asc(), TeamMatch.start_time.asc())
.all()
)
matches_data = []
for tm in matches:
confirmed, total = tm.get_confirmed_count()
participant = TeamMatchParticipant.query.filter_by(
team_match_id=tm.id,
player_id=current_user.id,
).first()
matches_data.append(
{
'match': tm,
'participant_id': participant.id if participant else None,
'is_confirmed': participant.is_confirmed if participant else False,
'confirmed_count': confirmed,
'total_count': total,
}
)
team_data.append(
{
'team': org_team,
'matches': matches_data,
'coaches': org_team.get_coaches(),
'managers': org_team.get_managers(),
}
)
return render_template('pages/my_teams.html', team_data=team_data, now=now)
def _posted(schema):
"""Load a form through `schema`, or None when it will not load.
The five assignment routes below each answer a bad field with their own
flash and a redirect to the same page, so a shared "it did not validate"
return is enough; the field-level message is flashed on the way out.
"""
try:
return schema.load(form_payload())
except ValidationError as err:
flash_validation_errors(err)
return None
def _assignable(user, expected_class):
"""Whether this account may be given a role on a team.
Deactivated accounts were offered by the selects and accepted by the
routes. `is_active_account` is what stops someone logging in — a person
who has left the club — so putting them on a roster contradicts the one
control that says they are gone. The listings filtered it for coaches and
managers and not for players, two lines apart, which is how it went
unnoticed.
"""
return isinstance(user, expected_class) and bool(user.is_active_account)
def _staff_member(user_id, expected_class):
"""The user behind an id, only if they may hold the role being assigned.
Returns None for a missing id, an unknown id, an account of the wrong
role, or a deactivated one. The role check is the point (SEC-16): the id
comes from a `<select>` the browser rendered, so it is a value the client
chooses, and nothing checked it in two of the three places that used it.
A forged submission could therefore list a player among a team's coaches
— the same defect wave G fixed in `tryouts.py`, left standing here.
Defers to `_assignable` rather than repeating `isinstance`: two functions
in one file answering "may this account take this role" differently is
the shape of every defect this module has had.
Args:
user_id: Already an int or None, thanks to the schema.
expected_class: Coach or Manager.
Returns:
User | None: The account, when it may take the role.
"""
if not user_id:
return None
user = db.session.get(User, user_id)
return user if user and _assignable(user, expected_class) else None
@teams_bp.route('/create', methods=['POST'])
@login_required
def create_team():
"""Create a new organization team."""
if not current_user.can_manage_teams():
flash(_('You do not have permission to create teams.'), 'danger')
return redirect(url_for('teams.list_teams'))
try:
data = OrgTeamSchema().load(form_payload(list_fields=('coach_ids', 'manager_ids')))
except ValidationError as err:
flash_validation_errors(err)
return redirect(url_for('teams.list_teams'))
name = data['name']
if OrgTeam.query.filter_by(name=name).first():
flash(_('Team "%(name)s" already exists.', name=name), 'danger')
return redirect(url_for('teams.list_teams'))
coach = _staff_member(data['coach_id'], Coach)
manager = _staff_member(data['manager_id'], Manager)
team = OrgTeam(
name=name,
coach_id=coach.id if coach else None,
manager_id=manager.id if manager else None,
created_by=current_user.id,
)
db.session.add(team)
db.session.flush()
if coach:
team.coaches.append(coach)
if manager:
team.managers.append(manager)
db.session.commit()
flash(_('Team "%(name)s" created successfully!', name=name), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/edit', methods=['POST'])
@login_required
def edit_team(team_id):
"""Edit an existing organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('You do not have permission to edit this team.'), 'danger')
return redirect(url_for('teams.list_teams'))
try:
data = OrgTeamSchema().load(form_payload(list_fields=('coach_ids', 'manager_ids')))
except ValidationError as err:
flash_validation_errors(err)
return redirect(url_for('teams.list_teams'))
name = data['name']
if OrgTeam.query.filter(OrgTeam.name == name, OrgTeam.id != team_id).first():
flash(_('Team "%(name)s" already exists.', name=name), 'danger')
return redirect(url_for('teams.list_teams'))
team.name = name
if data['sync_staff'] == '1':
team.coaches = [
user for user in (_staff_member(cid, Coach) for cid in data['coach_ids']) if user
]
coach_list = team.coaches.all()
team.coach_id = coach_list[0].id if coach_list else None
team.managers = [
user for user in (_staff_member(mid, Manager) for mid in data['manager_ids']) if user
]
manager_list = team.managers.all()
team.manager_id = manager_list[0].id if manager_list else None
else:
# This branch never checked the role, while the one above did — the
# same file disagreeing with itself (SEC-16). _staff_member is the
# single answer now.
coach = _staff_member(data['coach_id'], Coach)
manager = _staff_member(data['manager_id'], Manager)
team.coach_id = coach.id if coach else None
team.manager_id = manager.id if manager else None
if coach and not team.coaches.filter_by(id=coach.id).first():
team.coaches.append(coach)
if manager and not team.managers.filter_by(id=manager.id).first():
team.managers.append(manager)
db.session.commit()
flash(_('Team "%(name)s" updated successfully!', name=name), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/delete', methods=['POST'])
@login_required
def delete_team(team_id):
"""Delete an organization team.
Two checks, not one, and not the one the audit recommended (SEC-AUTHZ-006).
The constat was right about the inconsistency: this was the only team
operation guarded by the global `can_manage_teams()` while the other
nine use `can_manage_this_org_team(team)`. It was wrong about the fix.
Simply swapping to the per-object check **widens** access — `Coach`
returns False for the global capability and True for its own teams, so
the swap would hand every coach the power to delete the team they coach,
along with its notes and its match history. The constat reasoned about
`Manager`, where both return True, and missed the role where they differ.
Requiring both preserves today's behaviour exactly (admins and managers
yes, coaches no) and still closes the debt the constat was about: the
day `Manager.can_manage_this_org_team` is narrowed — which it should be —
deletion narrows with it instead of staying the one way in.
"""
team = OrgTeam.query.get_or_404(team_id)
if not (current_user.can_manage_teams() and current_user.can_manage_this_org_team(team)):
flash(_('You do not have permission to delete teams.'), 'danger')
return redirect(url_for('teams.list_teams'))
name = team.name
# One transaction. This used to commit three times, so a failure at the
# third step left the tryouts detached and the players removed without
# the team being deleted — an inconsistent state nothing could undo.
#
# TeamNote.org_team_id and TeamMatch.org_team_id are NOT NULL, and were
# not handled at all: deleting a team that had ever been used raised
# IntegrityError. Contract.team_id and OneOnOneRequest.org_team_id are
# nullable, and the rows outlive the team, so they are only detached.
# Entities that only make sense as part of the team.
TeamNote.query.filter_by(org_team_id=team_id).delete(synchronize_session=False)
for team_match in TeamMatch.query.filter_by(org_team_id=team_id).all():
db.session.delete(team_match) # participants follow by cascade
TeamPlayer.query.filter_by(org_team_id=team_id).delete(synchronize_session=False)
# Entities that survive it.
Tryout.query.filter_by(target_org_team_id=team_id).update(
{'target_org_team_id': None}, synchronize_session=False
)
Contract.query.filter_by(team_id=team_id).update({'team_id': None}, synchronize_session=False)
OneOnOneRequest.query.filter_by(org_team_id=team_id).update(
{'org_team_id': None}, synchronize_session=False
)
db.session.delete(team)
db.session.commit()
flash(_('Team "%(name)s" deleted successfully.', name=name), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/add_coach', methods=['POST'])
@login_required
def add_coach(team_id):
"""Add a coach to an organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('Permission denied.'), 'danger')
return redirect(url_for('teams.list_teams'))
data = _posted(TeamStaffSchema())
if data is None:
return redirect(url_for('teams.list_teams'))
if not data['coach_id']:
flash(_('Please select a coach.'), 'danger')
return redirect(url_for('teams.list_teams'))
coach = db.session.get(User, data['coach_id'])
if not coach or not _assignable(coach, Coach):
flash(_('Only coaches can be assigned as coach.'), 'danger')
return redirect(url_for('teams.list_teams'))
if team.coaches.filter_by(id=coach.id).first():
flash(
_(
'%(username)s is already a coach of %(name)s.',
username=coach.username,
name=team.name,
),
'info',
)
return redirect(url_for('teams.list_teams'))
team.coaches.append(coach)
if not team.coach_id:
team.coach_id = coach.id
db.session.commit()
flash(
_('%(username)s added as coach of %(name)s.', username=coach.username, name=team.name),
'success',
)
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/add_manager', methods=['POST'])
@login_required
def add_manager(team_id):
"""Add a manager to an organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('Permission denied.'), 'danger')
return redirect(url_for('teams.list_teams'))
data = _posted(TeamStaffSchema())
if data is None:
return redirect(url_for('teams.list_teams'))
if not data['manager_id']:
flash(_('Please select a manager.'), 'danger')
return redirect(url_for('teams.list_teams'))
manager = db.session.get(User, data['manager_id'])
if not manager or not _assignable(manager, Manager):
flash(_('Only managers can be assigned as manager.'), 'danger')
return redirect(url_for('teams.list_teams'))
if team.managers.filter_by(id=manager.id).first():
flash(
_(
'%(username)s is already a manager of %(name)s.',
username=manager.username,
name=team.name,
),
'info',
)
return redirect(url_for('teams.list_teams'))
team.managers.append(manager)
if not team.manager_id:
team.manager_id = manager.id
db.session.commit()
flash(
_('%(username)s added as manager of %(name)s.', username=manager.username, name=team.name),
'success',
)
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/remove_coach', methods=['POST'])
@login_required
def remove_coach(team_id):
"""Remove a coach from an organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('Permission denied.'), 'danger')
return redirect(url_for('teams.list_teams'))
data = _posted(TeamStaffSchema())
if data is None:
return redirect(url_for('teams.list_teams'))
if data['coach_id']:
coach = db.session.get(User, data['coach_id'])
if coach and team.coaches.filter_by(id=coach.id).first():
team.coaches.remove(coach)
if team.coach_id == coach.id:
team.coach_id = None
else:
team.coaches = []
team.coach_id = None
db.session.commit()
flash(_('Coach removed from %(name)s.', name=team.name), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/remove_manager', methods=['POST'])
@login_required
def remove_manager(team_id):
"""Remove a manager from an organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('Permission denied.'), 'danger')
return redirect(url_for('teams.list_teams'))
data = _posted(TeamStaffSchema())
if data is None:
return redirect(url_for('teams.list_teams'))
if data['manager_id']:
manager = db.session.get(User, data['manager_id'])
if manager and team.managers.filter_by(id=manager.id).first():
team.managers.remove(manager)
if team.manager_id == manager.id:
team.manager_id = None
else:
team.managers = []
team.manager_id = None
db.session.commit()
flash(_('Manager removed from %(name)s.', name=team.name), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/add_player', methods=['POST'])
@login_required
def add_player(team_id):
"""Add a player to an organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('Permission denied.'), 'danger')
return redirect(url_for('teams.list_teams'))
data = _posted(TeamPlayerSchema())
if data is None:
return redirect(url_for('teams.list_teams'))
if not data['player_id']:
flash(_('Please select a player.'), 'danger')
return redirect(url_for('teams.list_teams'))
status = data['status']
player = db.session.get(User, data['player_id'])
if not player or not _assignable(player, Player):
flash(_('Can only assign players to teams.'), 'danger')
return redirect(url_for('teams.list_teams'))
existing = TeamPlayer.query.filter_by(player_id=player.id, org_team_id=team.id).first()
if existing:
flash(
_('%(username)s is already on %(name)s.', username=player.username, name=team.name),
'info',
)
return redirect(url_for('teams.list_teams'))
tp = TeamPlayer(player_id=player.id, org_team_id=team.id, status=status)
db.session.add(tp)
db.session.commit()
flash(_('%(username)s added to %(name)s!', username=player.username, name=team.name), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/remove_player/<int:player_id>', methods=['POST'])
@login_required
def remove_player(team_id, player_id):
"""Remove a player from an organization team."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('Permission denied.'), 'danger')
return redirect(url_for('teams.list_teams'))
player = User.query.get_or_404(player_id)
tp = TeamPlayer.query.filter_by(player_id=player_id, org_team_id=team_id).first()
if not tp:
flash(
_('%(username)s is not on %(name)s.', username=player.username, name=team.name),
'danger',
)
return redirect(url_for('teams.list_teams'))
db.session.delete(tp)
db.session.commit()
flash(
_('%(username)s removed from %(name)s.', username=player.username, name=team.name),
'success',
)
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/toggle_status/<int:player_id>', methods=['POST'])
@json_endpoint
@login_required
def toggle_player_status(team_id, player_id):
"""Toggle a player's status between starter and substitute."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
return jsonify({'error': 'Permission denied'}), 403
tp = TeamPlayer.query.filter_by(player_id=player_id, org_team_id=team_id).first()
if not tp:
return jsonify({'error': 'Player not found on this team'}), 404
tp.status = 'substitute' if tp.status == 'starter' else 'starter'
db.session.commit()
return jsonify(
{
'success': True,
'player_id': player_id,
'new_status': tp.status,
'player_name': tp.player.username,
}
)
@teams_bp.route('/<int:team_id>/add-team-note', methods=['POST'])
@login_required
def add_team_note(team_id):
"""Add a team improvement note (coaches only)."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('You do not have permission to add notes to this team.'), 'danger')
return redirect(url_for('teams.list_teams'))
content = request.form.get('content', '').strip()
if content:
note = TeamNote(org_team_id=team_id, coach_id=current_user.id, content=content)
db.session.add(note)
db.session.commit()
flash(_('Team notes added successfully!'), 'success')
return redirect(url_for('teams.list_teams'))
@teams_bp.route('/<int:team_id>/add-player-note/<int:player_id>', methods=['POST'])
@login_required
def add_player_note(team_id, player_id):
"""Add a personal note for a player (coaches only)."""
team = OrgTeam.query.get_or_404(team_id)
if not current_user.can_manage_this_org_team(team):
flash(_('You do not have permission to add notes to this team.'), 'danger')
return redirect(url_for('teams.list_teams'))
player = User.query.get_or_404(player_id)
if not isinstance(player, Player):
flash(_('Can only add notes for players.'), 'danger')
return redirect(url_for('teams.list_teams'))
tp = TeamPlayer.query.filter_by(player_id=player_id, org_team_id=team_id).first()
if not tp:
flash(
_('%(username)s is not on %(name)s.', username=player.username, name=team.name),
'danger',
)
return redirect(url_for('teams.list_teams'))
content = request.form.get('content', '').strip()
if content:
note = PersonalNote(player_id=player_id, coach_id=current_user.id, content=content)
db.session.add(note)
db.session.commit()
flash(_('Note added for %(username)s!', username=player.username), 'success')
return redirect(url_for('teams.list_teams'))