From 3b7b9182d78e5ce97994b20705031da4b2486d4f Mon Sep 17 00:00:00 2001
From: GGThed <mafiaggia@outlook.com>
Date: Tue, 11 Aug 2026 20:48:55 -0400
Subject: [PATCH] fix(authz): balayer le motif au lieu d attendre la passe
 suivante

Le commit precedent finissait teams.py en notant que le defaut venait d'une
correction appliquee a un seul endroit. Balayer les autres modules
immediatement, plutot que d'attendre qu'une passe d'audit les retrouve, a
sorti les deux derniers.

tryouts.register_player lisait int(request.form.get('player_id')) -- 500 sur
une valeur non numerique -- et verifiait le role sans regarder
is_active_account. Un compte desactive pouvait donc etre inscrit a une
selection.

users/contracts._selectable_players ne filtrait pas non plus les comptes
desactives dans sa branche non-coach : la liste de depot de contrat proposait
encore des gens partis du club. Un contrat est un document nominatif signe.

PlayerSelectionSchema porte desormais le champ, et TeamPlayerSchema en herite
en ajoutant son statut. Un schema partage est ce qui empeche le prochain
appelant d'etre oublie -- c'est precisement parce que chaque route avait le
sien, ecrit a la main, que la correction a du etre faite trois fois.

Verifie par mutation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 app/routes/tryouts.py         | 18 +++++++---
 app/routes/users/contracts.py |  4 ++-
 app/validators.py             | 16 +++++++--
 tests/test_tryout_rules.py    | 63 +++++++++++++++++++++++++++++++++++
 4 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/app/routes/tryouts.py b/app/routes/tryouts.py
index 4d375e9..4c2be4c 100644
--- a/app/routes/tryouts.py
+++ b/app/routes/tryouts.py
@@ -32,7 +32,7 @@ from app.models import (
     TryoutRegistration,
     User,
 )
-from app.validators import TryoutSchema
+from app.validators import PlayerSelectionSchema, TryoutSchema
 
 tryouts_bp = Blueprint('tryouts', __name__, url_prefix='/tryouts')
 
@@ -479,13 +479,21 @@ def register_player(tryout_id):
     if not current_user.can_manage_this_tryout(tryout):
         flash(_('Permission denied.'), 'danger')
         return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id))
-    player_id = request.form.get('player_id')
-    if not player_id:
+    try:
+        data = PlayerSelectionSchema().load(form_payload())
+    except ValidationError as err:
+        flash_validation_errors(err)
+        return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id))
+
+    if not data['player_id']:
         flash(_('Please select a player.'), 'danger')
         return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id))
 
-    player = User.query.get_or_404(int(player_id))
-    if not isinstance(player, Player):
+    # Same two checks as the team roster (SEC-16): the right role, and an
+    # account that has not been deactivated. The select this comes from now
+    # filters both, but the select is not the control.
+    player = db.session.get(User, data['player_id'])
+    if not player or not isinstance(player, Player) or not player.is_active_account:
         flash(_('Can only register players.'), 'danger')
         return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id))
 
diff --git a/app/routes/users/contracts.py b/app/routes/users/contracts.py
index 865bc8f..eda1c8b 100644
--- a/app/routes/users/contracts.py
+++ b/app/routes/users/contracts.py
@@ -38,7 +38,9 @@ def manageable_players():
             if player_ids
             else []
         )
-    return User.query.filter_by(role='player').order_by(User.username).all()
+    # is_active_account: a contract select that still lists people who have
+    # left the club invites filing paperwork against them (SEC-16).
+    return User.query.filter_by(role='player', is_active_account=True).order_by(User.username).all()
 
 
 @users_bp.route('/contracts')
diff --git a/app/validators.py b/app/validators.py
index 230773b..7c25b24 100644
--- a/app/validators.py
+++ b/app/validators.py
@@ -501,8 +501,15 @@ class TeamStaffSchema(StripMixin):
     )
 
 
-class TeamPlayerSchema(StripMixin):
-    """A player being put on a team roster, and where they stand on it."""
+class PlayerSelectionSchema(StripMixin):
+    """One player id, picked from a `<select>` the browser rendered.
+
+    Shared by every route that takes a player from a roster form — putting
+    someone on a team, registering them for a tryout. They all read
+    `int(request.form.get('player_id'))` directly, so they were all a 500 on
+    a non-numeric value, and they were all fixed one at a time (SEC-16).
+    Having one schema is what stops the next one being missed.
+    """
 
     player_id = fields.Integer(
         allow_none=True,
@@ -510,6 +517,11 @@ class TeamPlayerSchema(StripMixin):
         validate=validate.Range(min=1),
         error_messages={'invalid': _l('Invalid player selection.')},
     )
+
+
+class TeamPlayerSchema(PlayerSelectionSchema):
+    """A player being put on a team roster, and where they stand on it."""
+
     status = fields.String(
         load_default='starter',
         validate=validate.OneOf(TEAM_PLAYER_STATUSES, error=_l('Unknown roster status.')),
diff --git a/tests/test_tryout_rules.py b/tests/test_tryout_rules.py
index d7445fc..e5adf72 100644
--- a/tests/test_tryout_rules.py
+++ b/tests/test_tryout_rules.py
@@ -296,3 +296,66 @@ class TestEndedTryoutsAreClosed:
 
         with app.app_context():
             assert Match.query.count() == 0
+
+
+class TestManualRegistrationChecksTheAccount:
+    """SEC-16, the last two places the pattern survived.
+
+    `register_player` read `int(request.form.get('player_id'))` — a 500 on a
+    non-numeric value — and checked the role but not whether the account had
+    been deactivated. The contract listing had the same gap in its player
+    select. Both were found by sweeping for the pattern after fixing
+    teams.py, rather than by waiting for the next audit pass to find them.
+    """
+
+    @pytest.fixture
+    def tryout_id(self, app, make_user):
+        from app.models import Tryout
+
+        admin_id = make_user('admin')
+        with app.app_context():
+            tryout = Tryout(
+                title='Spring',
+                game='Valorant',
+                date=date(2030, 4, 1),
+                created_by=admin_id,
+            )
+            db.session.add(tryout)
+            db.session.commit()
+            return tryout.id
+
+    def test_a_non_numeric_id_is_not_a_500(self, app, client, tryout_id, as_role):
+        as_role('admin')
+
+        response = client.post(
+            f'/tryouts/{tryout_id}/register_player', data={'player_id': 'not-a-number'}
+        )
+
+        assert response.status_code < 500
+
+    def test_an_active_player_registers(self, app, client, tryout_id, as_role, make_user):
+        """The premise: without it the test below passes against a route
+        that registers nobody."""
+        from app.models import TryoutRegistration
+
+        player_id = make_user('player')
+        as_role('admin')
+
+        client.post(f'/tryouts/{tryout_id}/register_player', data={'player_id': str(player_id)})
+
+        with app.app_context():
+            assert TryoutRegistration.query.filter_by(tryout_id=tryout_id).count() == 1
+
+    def test_a_deactivated_player_is_refused(self, app, client, tryout_id, as_role, make_user):
+        from app.models import TryoutRegistration, User
+
+        player_id = make_user('player')
+        with app.app_context():
+            db.session.get(User, player_id).is_active_account = False
+            db.session.commit()
+        as_role('admin')
+
+        client.post(f'/tryouts/{tryout_id}/register_player', data={'player_id': str(player_id)})
+
+        with app.app_context():
+            assert TryoutRegistration.query.filter_by(tryout_id=tryout_id).count() == 0