PERF-002, PERF-003, PERF-004. Aucun changement de comportement : chaque reecriture est accompagnee de tests qui enoncent la reponse attendue, pas la methode. PERF-002 -- /matches/api/events Le flux parcourait `tryout.matches` pour chaque tryout visible -- pour un president, tout l historique du club -- puis posait une requete MatchParticipant PAR match pour savoir si la personne qui regarde y figure. Le cout du calendrier croissait avec l historique, a chaque navigation. FullCalendar envoie deja `start` et `end` sur une source d evenements de type URL. Personne ne les lisait. La requete est desormais bornee, et les participants de tous les matchs de la fenetre sont charges en une fois, joueur compris. Des bornes illisibles sont ignorees plutot que refusees : un calendrier qui en montre trop est un probleme de performance, un calendrier qui renvoie 400 est une page blanche. PERF-003 -- get_players_available_at_time Chargeait tous les joueurs actifs, puis une requete PlayerDisponibility par joueur, sur une colonne non indexee. Deux requetes desormais, quelle que soit la taille du club. Mesure dans le test : 7 requetes pour 6 joueurs avant, 2 apres. PERF-004 -- decompte des evaluations en attente Chargeait toutes les inscriptions du club et toutes les evaluations du coach, construisait deux ensembles Python et les soustrayait -- deux lectures de table entiere pour produire un entier. Un COUNT DISTINCT avec NOT EXISTS. Les tests couvrent ce que la reecriture aurait pu changer sans bruit : fin de creneau exclusive, compte desactive exclu, joueur a cheval sur deux creneaux compte une fois, evaluation d un autre coach qui ne libere pas la ligne, double inscription comptee une fois (DB-006 n a pas encore atterri, donc le cas existe). PERF-001 (view_tryout) n est pas fait : c est le plus gros des quatre, il touche la page la plus consultee et merite son propre lot. 23 tests ajoutes, 371 au total. Co-Authored-By: Claude Opus 5 <[email protected]>
261 lines
9.3 KiB
Python
261 lines
9.3 KiB
Python
"""Contract of the calendar JSON API.
|
|
|
|
SEC-XSS-001. /matches/api/events used to build presentation markup into its
|
|
`description` field:
|
|
|
|
match_desc = participants_str + f"<br>{match.description}"
|
|
|
|
The browser dropped that value straight into innerHTML, so a player username
|
|
— free text at the time — reached the DOM of every coach and administrator
|
|
who opened the calendar. Escaping cannot be the browser's job alone here;
|
|
the API must not emit markup in the first place.
|
|
|
|
These tests assert the API contract. The DOM side is fixed in
|
|
calendar.html, which now builds nodes with textContent.
|
|
"""
|
|
|
|
from datetime import date, time
|
|
|
|
import pytest
|
|
|
|
from app.extensions import db
|
|
|
|
|
|
@pytest.fixture
|
|
def match_factory(app):
|
|
"""Create a tryout with one player_scrim match and one participant."""
|
|
from app.models import (
|
|
Match,
|
|
MatchParticipant,
|
|
Tryout,
|
|
TryoutRegistration,
|
|
)
|
|
|
|
def _make(owner_id, player_id, *, description=None, username=None):
|
|
from app.models import User
|
|
|
|
with app.app_context():
|
|
if username is not None:
|
|
# Written straight to the column: edit_profile now rejects
|
|
# this, but rows predating that fix can still hold anything.
|
|
player = db.session.get(User, player_id)
|
|
player.username = username
|
|
|
|
tryout = Tryout(
|
|
title='Spring tryout',
|
|
game='Valorant',
|
|
date=date(2030, 5, 1),
|
|
created_by=owner_id,
|
|
status='upcoming',
|
|
)
|
|
db.session.add(tryout)
|
|
db.session.flush()
|
|
|
|
db.session.add(TryoutRegistration(tryout_id=tryout.id, player_id=player_id))
|
|
|
|
match = Match(
|
|
tryout_id=tryout.id,
|
|
title='Scrim A',
|
|
description=description,
|
|
date=date(2030, 5, 2),
|
|
start_time=time(18, 0),
|
|
end_time=time(19, 0),
|
|
match_type='player_scrim',
|
|
created_by=owner_id,
|
|
)
|
|
db.session.add(match)
|
|
db.session.flush()
|
|
db.session.add(MatchParticipant(match_id=match.id, player_id=player_id))
|
|
db.session.commit()
|
|
return tryout.id, match.id
|
|
|
|
return _make
|
|
|
|
|
|
def _match_event(payload):
|
|
return next(e for e in payload if e['id'].startswith('match_'))
|
|
|
|
|
|
class TestCalendarEventPayload:
|
|
def test_description_is_returned_verbatim(self, app, client, as_role, make_user, match_factory):
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id, description='Bring your own peripherals')
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
assert props['description'] == 'Bring your own peripherals'
|
|
|
|
def test_description_does_not_carry_generated_markup(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
"""The <br> and the duplicated participant list are both gone."""
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id, description='Warm-up first')
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
assert '<br>' not in props['description']
|
|
assert props['participants'] not in props['description']
|
|
|
|
def test_an_empty_description_stays_empty(self, app, client, as_role, make_user, match_factory):
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id, description=None)
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
assert props['description'] == ''
|
|
|
|
def test_participants_remain_available_separately(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
"""Removing the concatenation must not lose the participant list:
|
|
the modal still renders it, from its own field."""
|
|
from app.models import User
|
|
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id, description='x')
|
|
|
|
with app.app_context():
|
|
username = db.session.get(User, player_id).username
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
assert props['participants'] == username
|
|
|
|
|
|
class TestLegacyHostileData:
|
|
"""Rows written before the username policy can still hold markup."""
|
|
|
|
PAYLOAD = '<img src=x onerror=alert(1)>'
|
|
|
|
def test_a_hostile_username_is_confined_to_its_own_field(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id, description='Normal text', username=self.PAYLOAD)
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
# The payload is data: it may legitimately appear in `participants`,
|
|
# which the client renders through textContent. What must never
|
|
# happen again is it being spliced into a field the client treats
|
|
# as markup.
|
|
assert props['description'] == 'Normal text'
|
|
assert self.PAYLOAD not in props['description']
|
|
|
|
|
|
class TestTheWindow:
|
|
"""PERF-002 — the feed used to return every match of every visible
|
|
tryout, which for a president is the club's whole history, and then ask
|
|
one more question per match to find out whether the viewer was in it.
|
|
|
|
FullCalendar sends `start` and `end` with a URL event source, so the
|
|
bounds were already arriving; nothing read them."""
|
|
|
|
def test_a_match_inside_the_window_is_returned(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id)
|
|
|
|
payload = client.get(
|
|
'/matches/api/events?start=2030-05-01T00:00:00-04:00&end=2030-05-31T00:00:00-04:00'
|
|
).get_json()
|
|
|
|
assert _match_event(payload)['date'] == '2030-05-02'
|
|
|
|
def test_a_match_outside_the_window_is_not(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id)
|
|
|
|
payload = client.get('/matches/api/events?start=2031-01-01&end=2031-01-31').get_json()
|
|
|
|
assert [e for e in payload if e['id'].startswith('match_')] == []
|
|
|
|
def test_no_bounds_still_returns_everything(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
"""Bookmarks, the tryout detail page and anything else calling this
|
|
without bounds must keep working."""
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id)
|
|
|
|
payload = client.get('/matches/api/events').get_json()
|
|
|
|
assert _match_event(payload)['date'] == '2030-05-02'
|
|
|
|
def test_unparseable_bounds_are_ignored_rather_than_rejected(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
"""A calendar showing too much is a performance problem; one that
|
|
returns 400 is a blank page."""
|
|
player_id = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, player_id)
|
|
|
|
response = client.get('/matches/api/events?start=whenever&end=')
|
|
|
|
assert response.status_code == 200
|
|
assert _match_event(response.get_json())['date'] == '2030-05-02'
|
|
|
|
def test_the_viewers_own_participation_still_comes_back(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
"""Batching the participant lookup must not lose the one row the
|
|
modal uses to offer 'confirm attendance'."""
|
|
admin_id = make_user('admin')
|
|
player_id = as_role('player')
|
|
_tryout_id, match_id = match_factory(admin_id, player_id)
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
assert props['user_participant_id'] is not None
|
|
assert props['user_attendance_confirmed'] is False
|
|
assert props['match_id'] == match_id
|
|
|
|
def test_someone_elses_participation_is_not_reported_as_yours(
|
|
self, app, client, as_role, make_user, match_factory
|
|
):
|
|
other_player = make_user('player')
|
|
admin_id = as_role('admin')
|
|
match_factory(admin_id, other_player)
|
|
|
|
props = _match_event(client.get('/matches/api/events').get_json())['extendedProps']
|
|
|
|
assert props['user_participant_id'] is None
|
|
|
|
|
|
class TestTheWindowParser:
|
|
def test_it_reads_the_date_out_of_an_iso_timestamp(self, app):
|
|
from datetime import date as date_type
|
|
|
|
from app.routes.matches import calendar_window
|
|
|
|
with app.test_request_context(
|
|
'/matches/api/events?start=2030-05-01T00:00:00-04:00&end=2030-05-31T23:59:59-04:00'
|
|
):
|
|
from flask import request
|
|
|
|
assert calendar_window(request.args) == (
|
|
date_type(2030, 5, 1),
|
|
date_type(2030, 5, 31),
|
|
)
|
|
|
|
def test_missing_or_broken_bounds_read_as_absent(self, app):
|
|
from app.routes.matches import calendar_window
|
|
|
|
with app.test_request_context('/matches/api/events?start=nope'):
|
|
from flask import request
|
|
|
|
assert calendar_window(request.args) == (None, None)
|