GGThedandClaude Opus 5
afab7070fb
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]>
103 lines
3.2 KiB
Python
103 lines
3.2 KiB
Python
"""Shared access-control rules.
|
|
|
|
Authorisation logic currently lives inline in eight route modules, and the
|
|
same question — "may this coach act on this player?" — is answered
|
|
differently depending on which route you reach. This module is the first
|
|
step towards a single point of truth (ARCH-002); rules move here as they
|
|
are unified, rather than in one sweeping change.
|
|
|
|
An important subtlety this module hides from callers: a coach can be
|
|
attached to a team two different ways.
|
|
|
|
OrgTeam.coach_id the original single-coach column
|
|
OrgTeam.coaches the many-to-many relationship added later
|
|
|
|
Both are still populated, and route code reads one or the other with no
|
|
apparent pattern. Reading only `coach_id` — which most of users.py does —
|
|
silently locks out every coach who is not the first one on their team.
|
|
coach_org_team_ids() always considers both.
|
|
"""
|
|
|
|
from app.extensions import db
|
|
|
|
|
|
def coach_org_team_ids(coach):
|
|
"""IDs of the organisation teams a coach is attached to.
|
|
|
|
Considers the many-to-many relationship *and* the legacy column, so the
|
|
second coach of a team is not treated as belonging to nothing.
|
|
|
|
Args:
|
|
coach: The user to inspect.
|
|
|
|
Returns:
|
|
list[int]: Team IDs, possibly empty.
|
|
"""
|
|
from app.models import OrgTeam
|
|
|
|
teams = OrgTeam.query.filter(
|
|
db.or_(
|
|
OrgTeam.coaches.any(id=coach.id),
|
|
OrgTeam.coach_id == coach.id,
|
|
)
|
|
).all()
|
|
return [team.id for team in teams]
|
|
|
|
|
|
def coach_can_access_player(coach, player_id):
|
|
"""Whether a coach may read or write information about a player.
|
|
|
|
True when the player sits on one of the coach's teams, or takes part in
|
|
a tryout the coach manages. Anything else means the two have no working
|
|
relationship, and a note or a contract about that player is none of the
|
|
coach's business.
|
|
|
|
Args:
|
|
coach: The acting coach.
|
|
player_id: Primary key of the player concerned.
|
|
|
|
Returns:
|
|
bool
|
|
"""
|
|
from app.models import (
|
|
Match, MatchParticipant, TeamPlayer, Tryout, TryoutRegistration,
|
|
)
|
|
|
|
if not player_id:
|
|
return False
|
|
|
|
team_ids = coach_org_team_ids(coach)
|
|
if team_ids:
|
|
on_team = TeamPlayer.query.filter(
|
|
TeamPlayer.player_id == player_id,
|
|
TeamPlayer.org_team_id.in_(team_ids),
|
|
).first()
|
|
if on_team:
|
|
return True
|
|
|
|
# Tryouts the coach manages, through any of the three routes the model
|
|
# supports: target team, many-to-many, or the deprecated coach_id.
|
|
conditions = [
|
|
Tryout.coaches.any(id=coach.id),
|
|
Tryout.coach_id == coach.id,
|
|
]
|
|
if team_ids:
|
|
conditions.append(Tryout.target_org_team_id.in_(team_ids))
|
|
|
|
tryout_ids = [t.id for t in Tryout.query.filter(db.or_(*conditions)).all()]
|
|
if not tryout_ids:
|
|
return False
|
|
|
|
registered = TryoutRegistration.query.filter(
|
|
TryoutRegistration.player_id == player_id,
|
|
TryoutRegistration.tryout_id.in_(tryout_ids),
|
|
).first()
|
|
if registered:
|
|
return True
|
|
|
|
plays_a_match = MatchParticipant.query.join(Match).filter(
|
|
MatchParticipant.player_id == player_id,
|
|
Match.tryout_id.in_(tryout_ids),
|
|
).first()
|
|
return plays_a_match is not None
|